Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Deprecate BoxStyle._Base.#17737
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| def __call__(self, x0, y0, width, height, mutation_size, | ||
| aspect_ratio=1.): | ||
| return self(self, x0, y0, width, height, mutation_size, 1) |
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.
Won't this fail with the wrong error if__call__ is not overridden, since it no longer takes anaspect_ratio argument?
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.
yes, but subclasses previously needed to override__call__ to get any meaningful behavior, no? previously not overriding it would result in a NotImplementedError from the base class, now you'd get a TypeError at argument binding time but I think that's close enough?
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.
I thought they had to overridetransmute before?
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.
Sorry, I got it wrong just above. Users had to overridetransmute before, which means that the implementation of _Base.transmute is in facto mostly irrelevant for backcompat purposes. I guess I could even leave it as raising a NotImplementedError? (although I would change the error message, which would be confusing)
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.
OK, but now nothing callstransmute? So how would existing subclassers find out the deprecation?
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.
hum... I guess that in__call__ I can add a call to_deprecate_method_override(__class__.transmute, ...) (and put back the old version of the code under theif transmute := _deprecate_method_override(...) branch)?
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.
Yea, if that works, sounds good.
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.
New version in, but the whole thing's a bit tricky...
Uh oh!
There was an error while loading.Please reload this page.
b109672 to83f6a53CompareUh oh!
There was an error while loading.Please reload this page.
| def __call__(self, x0, y0, width, height, mutation_size, | ||
| aspect_ratio=1.): | ||
| return self(self, x0, y0, width, height, mutation_size, 1) |
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.
OK, but now nothing callstransmute? So how would existing subclassers find out the deprecation?
... by moving the mutation_aspect logic to FancyBboxPatch, so thatboxstyles can be plain classes just implementing `__call__` and notinheriting from anything, which removes the need to inherit from aprivate base in `userdemo/custom_boxstyle01.py`.A similar approach could be implemented for FancyArrowStyle._Base andConnectionStyle._Base.
If you revert the changes to |
oops, |
@anntzer, I was going through removing 3.4 deprecations and I wasn't entirely clear on what you had planned for the final state here. This looks like it can be removed in main now if you're interested in taking it out though. |
anntzer commentedJun 10, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Done at#23237. Let's discuss it there. |
... by moving the mutation_aspect logic to FancyBboxPatch, so that
boxstyles can be plain classes just implementing
__call__and notinheriting from anything, which removes the need to inherit from a
private base in
userdemo/custom_boxstyle01.py.A similar approach could be implemented for FancyArrowStyle._Base and
ConnectionStyle._Base.
Closes#13199 (comment).
PR Summary
PR Checklist