fix MultiStrategyDispatch to work with new GenConverter and attrs inheritance #117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eritance Because the GenConverter specifically handles generating code for attrs classes, and because it registers hooks that don't require function dispatch, singledispatch was preventing subclasses from having code generated for them, because they would trigger the previously-generated base class's structure/unstructure code. This changes MultiStrategyDispatch to allow for a class-based hook to specifically avoid single-dispatch. Since we know that a given attrs class can always have code generated for it, we don't really want to share dispatch across subclasses - like the original Converter, we want to make sure we're structuring each individual attrs class as its own separate type.
petergaultney
force-pushed
the
bugfix/genconverter-inheritance
branch
from
a25ca9a to
285b608
Compare
Codecov Report
@@ Coverage Diff @@
## master #117 +/- ##
==========================================
+ Coverage 98.23% 98.25% +0.02%
==========================================
Files 7 7
Lines 453 459 +6
==========================================
+ Hits 445 451 +6
Misses 8 8
Continue to review full report at Codecov.
|
Tinche
requested changes
Jan 24, 2021
Tinche
reviewed
Jan 24, 2021
|
i put the entry in 1.2.0 but if you decide to do a bugfix release first, let me know and I can change it to a 1.1.3. |
petergaultney
force-pushed
the
bugfix/genconverter-inheritance
branch
from
02d4793 to
a8f28df
Compare
Tinche
approved these changes
Jan 25, 2021
|
I had to tweak this a little so that direct dispatch is consulted after the singledispatch, not before, to get some tests to pass. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because the GenConverter specifically handles generating code for
attrs classes, and because it registers hooks that don't require
function dispatch, singledispatch was preventing subclasses from
having code generated for them, because they would trigger the
previously-generated base class's structure/unstructure code.
This changes MultiStrategyDispatch to allow for a class-based hook to
specifically avoid single-dispatch. Since we know that a given attrs
class can always have code generated for it, we don't really want to
share dispatch across subclasses - like the original Converter, we
want to make sure we're structuring each individual attrs class as its
own separate type.
fixes #116