-
Notifications
You must be signed in to change notification settings - Fork 51
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
Handle typing.Generic case in MRO #847
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #847 +/- ##
==========================================
- Coverage 92.79% 92.79% -0.01%
==========================================
Files 47 47
Lines 8468 8491 +23
Branches 1550 1555 +5
==========================================
+ Hits 7858 7879 +21
- Misses 350 351 +1
- Partials 260 261 +1 ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This is not the most correct implementation. Sorting the MRO is not the way to go. I’ll think about it more but we might have to refactor the whole MRO computing code… |
…ould be adjustable for future tweaks like the one for typing.Generic.
…ng-Generic-in-MRO # Conflicts: # pydoctor/model.py
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Diff from pydoctor_primer, showing the effect of this PR on open source code: twisted (https://github.com/twisted/twisted)
- /projects/twisted/src/twisted/python/zippath.py:52: Cannot compute linearization of the class inheritance hierarchy
- /projects/twisted/src/twisted/python/zippath.py:261: Cannot compute linearization of the class inheritance hierarchy
|
assert capsys.readouterr().out == '''mro:31: Cannot compute linearization of the class inheritance hierarchy of 'mro.Duplicates' | ||
mro:9: Cannot compute linearization of the class inheritance hierarchy of 'mro.F1' | ||
mro:10: Cannot compute linearization of the class inheritance hierarchy of 'mro.G1' | ||
''' |
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.
It would be good to have a test that ensure that a cycle in one of the base classes does not prevent to compute unrelated class’s MROS
Fix #846.
This should also speed-up the MRO processing since we now process all classes in topological order and use caching for predecessor MROs, avoiding to re-compute for every subclasses like the current code does.