Closed jharwell closed 3 years ago
@jharwell thanks for providing details on exactly how your code is structured.
Could you try the following and let me know if the issue still persists?
Upgrade and test with the latest version of implements (0.2.0). There is only one occurrence of cls_method = getattr(cls, name...
in implements.py, and it is at this location in the latest master. The code now has cls_method = getattr(cls, name, None)
, which has the same effect as the fix you've suggested.
In the file _/projects/myproject/models/homingtime.py:
@implements.implements(models.interface.IModel)
class IntraExpNestHomingTime1Robot(models.interface.IModel): <--- Avoid inheriting from the Interface class
def run(self, cmdopts: dict, exp_num: int) -> tp.List[pd.DataFrame]:
pass
def target_csv_stems(self) -> tp.List[str]:
pass
Please change this to:
@implements.implements(models.interface.IModel)
class IntraExpNestHomingTime1Robot:
def run(self, cmdopts: dict, exp_num: int) -> tp.List[pd.DataFrame]:
pass
def target_csv_stems(self) -> tp.List[str]:
pass
Inheriting from the interface must be avoided. This doesn't pose a problem with your current example though, as your implementation class (IntraExpNestHomingTime1Robot
) ends up overriding the two methods from the interface (models.interface.IModel
).
However, if the interface class defines a method which the implementation class has missed implementing, such a miss will not get flagged. This is because the method from the interface gets inherited into the implementation.
Why the behavior of getattr() is different in the two cases, I have no idea. I also have no idea why the run() function works correctly even in this use case with the current implements implementation (though it could be some subtle declaration order thing I haven't figured out yet). Overal, I think this is a bug in implements, provided I am using it correctly in the stripped down example above.
I don't have an answer at this point on the above. I tried to reproduce the issue in a manner similar to your structure, but I couldn't observe the issue. PS: I don't know the version of implements you are using. Perhaps there are far more changes between our environments than just the line cls_method = getattr(...)
.
Here are some more insights (FYI):
The file homing_time.py
imports models.interface
the usual way. The interface enforcement occurs at the line @implements.implements(models.interface.IModel)
. However, in your current case, the enforcement logic is acting on the implementation which already inherits from the interface (as in pt.2).
From the perspective of main.py
, the enforcement must have already taken place at spec.loader.exec_module(module)
, and you should expect to see interface errors if any, even if model = getattr(module, 'IntraExpNestHomingTime1Robot')
is not present.
Please do try out the two things mentioned above. I look forward to hearing from you. Thanks 👍
Ah I see--avoiding inheritance makes sense. I come from a C++ background, where inheriting interfaces is how you get the compiler to enforce conformance to said interfaces for you. I'm currenly running version 0.2.0 (the pypi version). If I remove the inheritance, then things work! Looking at my non-dynamic module importing, implements worked there because I used it properly and did not inherit from interfaces, (it had been a while since I looked at that code, so I just assumed that I was using implements the same everywhere).
Perhaps adding a few lines of documentation to implements emphasizing that the @implements decorator is mutually exclusive with inheriting from an interface class which has pure virtual/undefined methods ? That might help future users.
So in summary, this was not a bug in implements--I was just using it incorrectly.
@jharwell glad to know. Thanks.
Enforcement via inheritance(+metaclasses) are a common pattern in some python libraries too. Yep. Agree with you on gap in educating the user about the implementation class not inheriting the interface class.
In fact, implements could go a step further, inspect the MRO and verify that the class hierarchies of the implementation and the interface classes are disjoint sets.
@jharwell would it be OK if I closed this issue, and opened an enhancement request for the implementation class not inheriting from the interface?
Yes definitely. :+1:
I have the following situation: