Skip to content
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

BUG: make sure find_lowest_subclasses doesn't add extra classes #4404

Merged
merged 1 commit into from
Apr 8, 2023

Conversation

yut23
Copy link
Member

@yut23 yut23 commented Apr 7, 2023

PR Summary

If a child class and its grandparent (or another ancestor) are the only classes in candidates, the previous implementation of find_lowest_subclasses would return both the child class and the parent class. This PR changes the logic so that it will only return classes passed in as candidates.

Needed by #4402.

>>> import inspect
>>> from collections import Counter
>>> from functools import reduce

>>> class Grandparent:
...     pass
... class Parent(Grandparent):
...     pass
... class Child(Parent):
...     pass

>>> candidates = [Grandparent, Child]
>>> mros = [inspect.getmro(c) for c in candidates]; mros
[(__main__.Grandparent, object),
 (__main__.Child, __main__.Parent, __main__.Grandparent, object)]

>>> counters = [Counter(mro) for mro in mros]; counters
[Counter({__main__.Grandparent: 1,
          object: 1}),
 Counter({__main__.Child: 1,
          __main__.Parent: 1,
          __main__.Grandparent: 1,
          object: 1})]

# Parent only appeared once, so the original code will include it
>>> count = reduce(lambda x, y: x + y, counters); count
Counter({__main__.Grandparent: 2,
         object: 2,
         __main__.Child: 1,
         __main__.Parent: 1})

>>> [x for x in count.keys() if count[x] == 1]
[__main__.Child, __main__.Parent]

>>> [x for x in candidates if count[x] == 1]
[__main__.Child]

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.

If a child class and its grandparent are both in candidates, the
previous implementation would return both the child class and the parent
class.
@neutrinoceros
Copy link
Member

thank you for providing context. LGTM !

@neutrinoceros neutrinoceros changed the title FIX: make sure find_lowest_subclasses doesn't add extra classes BUG: make sure find_lowest_subclasses doesn't add extra classes Apr 8, 2023
@neutrinoceros neutrinoceros merged commit c29aae4 into yt-project:main Apr 8, 2023
@yut23 yut23 deleted the fix_find_lowest_subclasses branch May 18, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants