-
-
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
Fix/callable protocols 2 #599
base: main
Are you sure you want to change the base?
Fix/callable protocols 2 #599
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #599 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 6492 6493 +1
=========================================
+ Hits 6492 6493 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a unit test for a protocol with __call__
.
Fixed | ||
^^^^^ | ||
- Fix callable protocol inheritance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v4.34.0 hasn't been released yet, so this entry should be there instead of a new v4.34.1 section. Also please add a link to this pull request.
from jsonargparse._parameter_resolvers import get_signature_parameters | ||
from jsonargparse._postponed_annotations import get_return_type | ||
|
||
if not inspect.isclass(value): | ||
return False | ||
members = 0 | ||
for name, _ in inspect.getmembers(protocol, predicate=inspect.isfunction): | ||
if name.startswith("_"): | ||
if name.startswith("_") and name not in allowed_dunder_methods: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Give me some time and I analyze the dunder methods in general and maybe in this pull request we do more than __call__
.
Quality Gate passedIssues Measures |
I added some tests for callable protocols. One question that came up is whether functions should be checked against the protocol. Currently that test is failing, here is the example:
The result is:
|
What does this PR do?
Fixes callable protocol inheritance by allowing a list of private methods to be considered when matching signatures for inheritance of subclasses. The set of
allowed_dunder_methods
currently only contains the__call__
method.Fixes #595
Before submitting