Skip to content

Conversation

@sthagen
Copy link
Owner

@sthagen sthagen commented Jul 23, 2021

…ns (python#10844)

  • Add test

Add a test that makes sure that our implementation goes up the MRO of
the dispatch type and uses the registered implementation that comes
first, instead of going through implementations in the reverse order.

  • Check subclasses before their superclasses

When creating a dispatch function for a singledispatch function, avoid
checking any classes before we've checked all of their subclasses. That
makes sure we use the implementation with the dispatch type that appears
first in the actual argument's type's MRO, which is what the standard
library implementation of singledispatch does (it goes up the argument's
MRO, looking for classes that have registered implementations associated
with them).

  • Add test to reproduce incorrect class sorting

Add a test to reproduce the bug in the class sorting implementation
mentioned in
python#10844 (comment).

  • Improve class sorting implementation

Improve the sorting of classes to:

  • always put subclasses before any of their superclasses, even when
    there are multiple related classes (see
    testMultipleRelatedClassesBeingRegistered for an example of this)
  • preserve the order of implementations with unrelated dispatch types
  • Remove unused Iterable import

The one use of Iterable was removed in the previous commit, so this is
unnecessary.

  • Change class sorting to use topological sort

Change the sorting of classes that we use when generating dispatch
functions to use topological sort on a graph made up of the classes with
edges pointing from subclasses to the classes on their MRO.

This also modifies the signature of topsort to take any type for the
vertices of the graph.

  • Create graph with a dictionary comprehension

Use a dictionary comprehension to make the graph creation nicer.

  • Remove unnecessary imports

Have you read the Contributing Guidelines?

(Once you have, delete this section. If you leave it in, your PR may be closed without action.)

Description

(Explain how this PR changes mypy.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

…ns (#10844)

* Add test

Add a test that makes sure that our implementation goes up the MRO of
the dispatch type and uses the registered implementation that comes
first, instead of going through implementations in the reverse order.

* Check subclasses before their superclasses

When creating a dispatch function for a singledispatch function, avoid
checking any classes before we've checked all of their subclasses. That
makes sure we use the implementation with the dispatch type that appears
first in the actual argument's type's MRO, which is what the standard
library implementation of singledispatch does (it goes up the argument's
MRO, looking for classes that have registered implementations associated
with them).

* Add test to reproduce incorrect class sorting

Add a test to reproduce the bug in the class sorting implementation
mentioned in
#10844 (comment).

* Improve class sorting implementation

Improve the sorting of classes to:
- always put subclasses before any of their superclasses, even when
  there are multiple related classes (see
  testMultipleRelatedClassesBeingRegistered for an example of this)
- preserve the order of implementations with unrelated dispatch types

* Remove unused Iterable import

The one use of Iterable was removed in the previous commit, so this is
unnecessary.

* Change class sorting to use topological sort

Change the sorting of classes that we use when generating dispatch
functions to use topological sort on a graph made up of the classes with
edges pointing from subclasses to the classes on their MRO.

This also modifies the signature of topsort to take any type for the
vertices of the graph.

* Create graph with a dictionary comprehension

Use a dictionary comprehension to make the graph creation nicer.

* Remove unnecessary imports
@sthagen sthagen merged commit cf8a7b5 into sthagen:master Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants