Skip to content

FIX: MouseButton representation in boilerplate generated signatures #20148

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

Merged
merged 1 commit into from
May 4, 2021

Conversation

tacaswell
Copy link
Member

PR Summary

In Python 3.10 the repr and str representation of Enums changed from

str: 'ClassName.NAME' -> 'NAME'
repr: '<ClassName.NAME: value>' -> 'ClassName.NAME'

which is more consistent with what str/repr should do, however this breaks
boilerplate which needs to get the ClassName.NAME version in all versions of
Python. Thus, we locally monkey patch our preferred str representation in
here.

bpo-40066
python/cpython#22392

Other options considered:

  • patch MouseButton.__str__ in matplotlib.backend_bases. This has the advantage of maintaining stability of str(MouseButton.Left) over time, but this feels like something we should not fight upstream on. If they chose to break this intentionally we should follow them and not make our Enum "special"
  • re-writing
    signature = str(signature.replace(parameters=[
    param.replace(default=value_formatter(param.default))
    if param.default is not param.empty else param
    for param in params]))
    to special case MouseButton, but that would more-or-less involve vendoring the str methods from both inspect.Signature and inspect.Parameter which are about a screen of code in total.

PR Checklist

  • Has pytest style unit tests (and pytest passes). (this fixes a failure on py310)
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • [N/A] New features are documented, with examples if plot related.
  • [N/A] Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • [N/A] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@tacaswell tacaswell added this to the v3.4.2 milestone May 4, 2021
@@ -25,6 +25,28 @@
from matplotlib import _api, mlab
from matplotlib.axes import Axes
from matplotlib.figure import Figure
from matplotlib.backend_bases import MouseButton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

order imports alphabetically

Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit

In Python 3.10 the repr and str representation of Enums changed from

 str: 'ClassName.NAME' -> 'NAME'
 repr: '<ClassName.NAME: value>' -> 'ClassName.NAME'

which is more consistent with what str/repr should do, however this breaks
boilerplate which needs to get the ClassName.NAME version in all versions of
Python. Thus, we locally monkey patch our preferred str representation in
here.

bpo-40066
python/cpython#22392
@tacaswell tacaswell force-pushed the fix_py310_boilerplate_compat branch from 97d07b8 to 93973ab Compare May 4, 2021 13:12
@timhoffm timhoffm merged commit 48fbf56 into matplotlib:master May 4, 2021
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request May 4, 2021
timhoffm added a commit that referenced this pull request May 4, 2021
…148-on-v3.4.x

Backport PR #20148 on branch v3.4.x (FIX: MouseButton representation in boilerplate generated signatures)
@tacaswell tacaswell deleted the fix_py310_boilerplate_compat branch May 4, 2021 22:14
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.

3 participants