python-bindings
python-bindings copied to clipboard
Name of API function/class "Interface" can be misleading
The class Interface
https://github.com/precice/python-bindings/blob/85245af97288c54b53d8ce26067408218aa99a0d/cyprecice/cyprecice.pyx#L34
can be misleading. It also deviates from the naming in the C++ API where the equivalent class is called SolverInterface
.
This came up on Gitter where a user had troubles to create two interfaces for his coupling. They needed two meshes which they interpreted as two coupling interfaces and connected that to the Interface
object in the Python bindings. I do not think that their way of thinking was completely wrong. I think that misunderstanding got even worse due to the usage of the term "interface" in the OpenFOAM adapter as this adapter was used by the user as well.
There might be additional (other) usage of the term "interface" in other adapters.
This problem does not appear all the time, but maybe we should discuss how one could avoid this misunderstanding.
I totally agree. I at least cannot remember any real "reasoning" behind picking the term Interface
over SolverInterface
(which would be the reasonable default). Probably it was just always Interface
and we never touched it.
A remark on implementation: We can still provide Interface
as a deprecated wrapper of SolverInterface
then we can push when this change actually breaks user code until v3.0.0.1
, if we want.
Some initial trials of changing the main API class name from Interface
to SolverInterface
shed light on the following issue:
The class SolverInterface
already exists as the wrapper cppclass
:
https://github.com/precice/python-bindings/blob/85245af97288c54b53d8ce26067408218aa99a0d/cyprecice/SolverInterface.pxd#L6-L12
Hence using the class name again is not allowed. Renaming the SolverInterface
class in the C++ wrapper conflicts with the original class name in the C++ API of preCICE. I assume this was the logic behind naming the Python class Interface
and not SolverInterface
. One solution could be wrap the C++ wrapper class into another class with a different name, like for example PRECICESolverInterface
and then call that class pointer from the the renamed Python class SolverInterface
.
Would it be worthwhile to do this?
In 86f249d @IshaanDesai and I implemented a workaround to avoid wrong usage. Replacing SolverInterface
with Interface
is possible, but at least to me currently not straightforward. I would suggest to postpone this issue for now, but anybody who has time to fix it, is, of course, invited to contribute :smirk:
Let's quickly summarize the ideas we had and problems we saw or expected:
- Just writing
from cyprecice import Interface as SolverInterface
works as a wrapper, but then the name in the documentation staysInterface
- Renaming
Interface
toSolverInterface
here causes a name clash. - We could also rename
cyprecice/SolverInterface.pxd
tocyprecice/cySolverInterface.pxd
, but this might again lead to problems with the includedSolverInterface.hpp
file that we cannot change. We did not try it, though.