Skip to content
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

TST: Backend switching API (don't merge) #11612

Closed
wants to merge 5 commits into from

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Jul 9, 2018

PR Summary

This PR creates a possible API for backend switching. The API is laid out in the last commit. Previous commits are from #11600 and only contain the actual switching logic.

The code is not fully tested. It's main purpose is to give an idea how the following API can be implemented:

  • There is a new rcParams['default_backends'] that accepts a list of possible backends to try out.
  • The current behavior for setting the default backend via rcParams['backend'] is deprecated.
  • Reading rcParams['backend'] is deprecated. Users should use rcParams['default_backends']orget_backend()` depeding on their needs.
  • matplotlib.use() is deprecated in favor of set_backend() -- this is independent of the other API changes but adds to consistency.

Typical use-cases:

Setting the default backend

new API:

rcParams['default_backends'] = ['qt5agg', 'qt4agg']

current API (deprecated):

rcParams['backend'] = 'qt5agg'  # only one supported
WARNING: The rcParam 'backend' is deprecated. Use 'default_backends' instead.

Setting the current backend

new API:

matplotlib.set_backend('qt5agg')

current API (deprecated):

matplotlib.use('qt5agg')
WARNING: matplotlib.use is deprecated. Use matplotlib.set_backend instead.

Querying the current backend:

new API:

matplotlib.get_backend()

current API (deprecated):

backend = matplotlib.rcParams['backend']  # returns the current or default backend
WARNING: The rcParam 'backend' is deprecated. Use the rcParam 'default_backends' to access the defaults. Use get_backend() to get the current backend."

anntzer and others added 5 commits July 9, 2018 15:32
See changes documented in the API changes file.

Some followup cleanup (of the now unused old machinery) will come as a
separate PR (left some "FIXME: Remove." comments).

Changes to the build process (namely, getting rid of trying to detect
the default backend in setupext.py) will come as a separate PR.

I inlined pylab_setup into switch_backend (and deprecated the old
version of pylab_setup) because otherwise the typical call stack would
be `use()` -> `set rcParams['backend'] = ...` -> `switch_backend()` ->
`pylab_setup()`, which is a bit of a mess; at least we can get rid of
one of the layers.
The selection logic is now described in the module's docstring.  The
only changes is that the QT_ENV_MAJOR_VERSION global, which would
sometimes be defined (depending on the state of the import cache, the
QT_API environment variable, and the requested backend) is never defined
anymore.
@timhoffm timhoffm changed the title Backend switching API TST: Backend switching API (don't merge) Jul 9, 2018
@tacaswell
Copy link
Member

The matplotlib.use API has been there from 2004 (goes all the way back to e34a333) and it is in a lot of code / documentation / examples. I am not convinced that the slight improvement in the verbiage is worth disrupting a considerable fraction of our users. While if we were going to be starting from a clean slate I agree set_backend is a better name, I am 👎 on deprecating use.

I am 50/50 on adding set_backend as an alias for use, it is a better more guessable name, but now we have 2 versions of the call...

Similarly, storing the current backend in rcParams has been there as long, ex the current implementation of get_backend is:

def get_backend():
"""Return the name of the current backend."""
return rcParams['backend']

I think I am on board with pushing people to get_backend instead of rcParams['backend'], but that is going to need a long (multi-year) deprecation cycle.

I am 👍 on adding 'default_backends' as a parameter, but (as discussed on the call today) we may want to move things like backend, interactive, etc out of rcParams all together so we should have a plan for that before we put it in.

@tacaswell tacaswell added this to the v3.1 milestone Jul 10, 2018
@timhoffm
Copy link
Member Author

timhoffm commented Jul 10, 2018

The changes are rather independent and could be introduced incrementally, if we are not sure about some of them:

  1. Enabling backend switching itself does not need any new API. I can simply allow matplotlib.use() when there already is a backend.
  2. Introducing set_backend() is optional and can be done anytime as this is just an alias of matplotlib.use().
  3. Deprecating matplotlib.use() after introducing set_backend() is optional. I accept the argument that this would be disrupting existing users. One alternative approach would be a PendingDeprecation. This would tell new users, that they sould use set_backend() but could keep matplotlib.use() for a long time without nagging existing users with a deprecation warning.
    But it's also a fair decision to just keep matplotlib.use(). I just would not recommend to have both set_backend() and matplotlib.use() as pure aliases. There should be one recommended way, even if we allow both to exist.
  4. Deprecating querying rcParams['backend'] to get the current backend in favor of get_backend() is optional. Could also start with a PendingDeprecation.
  5. Introducing the support for a list of default backends is optional. I just recommend not to inject that functionality into rcParams['backend'], but first establish a distinct entity for holding the defaults (as opposed to the current value) - rcParams['default_backends'] would be one solution for this.

A reasonable minimal start for 3.0 could be just to do 1. (and maybe 4.) - acknowledging that the original PRs #11581 and #11600 are tagged 3.0 and release-critical. The other points seem to be more controversial and there is no need to rush it.

try:
switch_backend(candidate)
except ImportError:
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

probably need to do something if no backend is successfully imported.

cbook.warn_deprecated('3.0',
"The rcParam 'backend' is deprecated. Use the rcParam "
"'default_backends' to access the defaults. Use get_backend() "
"to get the current backend.")
Copy link
Contributor

Choose a reason for hiding this comment

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

But then you need to store the current backend somewhere else and update the implementation of get_backend accordingly.

@jklymak
Copy link
Member

jklymak commented Jul 10, 2018

@timhoffm I agree with your suggestions above. OTOH for now we want to get backend switching in, and the existing rcParam stuffing, while suboptimal, is the existing API. So for 3.0 I think we decided to move along w/ @anntzer PR (plus or minus any review changes) and then we can re-open the deprecation/API refinement for 3.1 (due December-ish).

@timhoffm
Copy link
Member Author

timhoffm commented Sep 2, 2018

Superseeded by #11600

@timhoffm timhoffm closed this Sep 2, 2018
@timhoffm timhoffm deleted the backend-switching-3 branch September 2, 2018 08:53
@QuLogic QuLogic removed this from the v3.1 milestone Sep 2, 2018
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.

5 participants