Skip to content

Simplify handling of Qt modifier keys. #14507

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 2 commits into from
Jun 9, 2020
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jun 8, 2019

PR Summary

  • Replace MODIFIER_KEYS by a simpler private table that doesn't include
    the names used by Matplotlib ("super"/"alt"/...), instead just get
    these names from SPECIAL_KEYS (this avoids having to also update
    _MODIFIER_KEYS for osx, where we use different names).
  • Delete the globals SUPER, ALT, CTLR, SHIFT which were just indices
    into MODIFIER_KEYS but otherwise meaningless.
  • Invert the order of _MODIFIERS_KEYS to avoid having to do a
    reverse() later in the code.
  • Get max_unicode value from Python's sys module.

No deprecation, as these can't be properly deprecated anyways.

Goes on top of #14506.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer
Copy link
Contributor Author

anntzer commented May 22, 2020

rebased

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

No deprecation, as these can't be properly deprecated anyways.

You can deprecate anything. It's just a matter of writing an entry into the deprecations list. What you cannot do here is warn the user when he uses the deprecated variables (well at least not before 3.7).

I may be overly catious but this seems like some people will have used it. So I'm in favor of a proper depreaction.

  • Add a deprecation note. We're at least formally safe. People could have read the deprecation notes before switching to a new version.
  • For Py3.7+ use module-level getattr to warn.


The ``MODIFIER_KEYS``, ``SUPER``, ``ALT``, ``CTRL``, and ``SHIFT`` global
variables of the :mod:`matplotlib.backends.backend_qt4` and
:mod:`matplotlib.backends.backend_qt5` modules have been removed.
Copy link
Member

@timhoffm timhoffm May 22, 2020

Choose a reason for hiding this comment

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

"Use the native Qt modifiers instead."

@anntzer
Copy link
Contributor Author

anntzer commented Jun 7, 2020

Restored with deprecation notice. Let's leave module-level getattr until we set up some generic helpers for them (I had a PR somewhere demo'ing how to do that...).

@anntzer anntzer added this to the v3.3.0 milestone Jun 7, 2020
@anntzer anntzer force-pushed the qtkeys branch 2 times, most recently from b850318 to c935bb7 Compare June 7, 2020 17:34
@timhoffm
Copy link
Member

timhoffm commented Jun 8, 2020

Doc build failures:

/home/circleci/project/doc/api/api_changes_3.3/deprecations.rst:599: WARNING: py:mod reference target not found: matplotlib.backends.backend_qt4
/home/circleci/project/doc/api/api_changes_3.3/deprecations.rst:599: WARNING: py:mod reference target not found: matplotlib.backends.backend_qt5

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Modulo doc build.

- Replace MODIFIER_KEYS by a simpler private table that doesn't include
  the names used by Matplotlib ("super"/"alt"/...), instead just get
  these names from SPECIAL_KEYS (this avoids having to also update
  _MODIFIER_KEYS for osx, where we use different names).
- Deprecate the globals SUPER, ALT, CTLR, SHIFT which were just indices
  into MODIFIER_KEYS but otherwise meaningless.
- Invert the order of _MODIFIERS_KEYS to avoid having to do a
  `reverse()` later in the code.
- Get max_unicode value from Python's sys module.
@QuLogic QuLogic merged commit 20da503 into matplotlib:master Jun 9, 2020
@anntzer anntzer deleted the qtkeys branch June 9, 2020 21:43
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.

3 participants