-
-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Callable protocols inheritance broken #595
Comments
Thank you for reporting! Indeed the intention was to not consider private methods when checking whether a protocol is implemented. This is here jsonargparse/jsonargparse/_typehints.py Lines 1096 to 1097 in 2825a73
Though, it was not intended to exclude if name.startswith("_") and name != "__call__": Do you want to contribute the fix? |
Thanks, yes sure I will fix it |
what is the reason for excluding private methods though? |
The purpose of a But note that dunder methods, such as |
I see your point, maybe we can have a list of allowed dunder methods which can be expanded when needed? However, I do think it might be out of the scope of the parser to try to enforce/disallow discouraged practices. Unless there is consistent rule about which dunder methods are allowed, it will be hard to predict the expected behavior of the parser. Is there any other downside to removing the check: jsonargparse/jsonargparse/_typehints.py Lines 1096 to 1097 in 2825a73
|
The good practices is a design decision, similar to the one for lightning. Also in practical terms, (not specific to this case) allowing unexpected uses, can mean that people implement and rely on behaviors which were not really intended, and add to the maintainance cost of the library. For sure allowing private methods will not happen. Dunder methods is something to analyze. |
Ok, makes sense. I created a PR which adds a set of exceptions for dunder methods and added |
🐛 Bug report
Callable protocols (implement
__call__
) seems to be parser incorrectly in comparison to "normal" protocols.To reproduce
Example that works
Example that does not work:
The second example raises the following error:
Seems like the parser does not match the inherited protocol signature to the base if the protocol methods are private (?).
Expected behavior
I would expect the second example to work like the first.
Environment
The text was updated successfully, but these errors were encountered: