Skip to content

Add __all__ to pyplot #12743

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add __all__ to pyplot #12743

wants to merge 1 commit into from
+138 −2

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Nov 4, 2018 •
edited
Loading

PR Summary

This introduces the __all__ variable to pyplot, whose only purpose is to limit what is imported on a wildcard import from matplotlib.pyplot import *.

While wildcard imports are not recommended in general - and I don't want to change this, there appears to be still a lot of from pylab import * or ipython --pylab usage around. I assume that's because it's so convenient for quick interactive use. While we probably cannot change people completely, it may be a good idea to have a more limited and thus less dangerous wildcard import to help push people away from pylab. In particular we should only import relevant plotting functions here (no numpy, no mlab, no colormap functions)

Note: This is currently a proof of concept and requires additional work.

  • This affects pylab since pylab uses from matplotlib.pyplot import *, which is an unintended side-effect.
  • The hard-coded part of functions in __all__ needs some cleanup. Possible some are missing, while others are not neccesary.

But before working futher on the details, I would like to reach a general consensus on the idea.

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

@timhoffm
Copy link
Member Author

timhoffm commented Nov 4, 2018

Note: CI for 3.5 is currently failing due to non-deterministic dict order. This can be worked around or defered until mpl 3.1, which will be python 3.6+ only. Not going to spend time on this before the general idea is discussed.

@timhoffm timhoffm added this to the v3.1 milestone Nov 4, 2018
@ImportanceOfBeingErnest
Copy link
Member

You can be certain that a lot of people do things like

axes().add_artist(Rectangle((.5,.5),.2,.3, color=cm.magma(.4)))

having started ipython --pylab.

@timhoffm
Copy link
Member Author

timhoffm commented Nov 4, 2018

Except for the cm.magma, which would have to be discussed, this is fine by me.

This is about defining a reasonable subset of pylab that should ease most use-cases while limiting the number of imported functions.

@timhoffm timhoffm modified the milestones: v3.1.0, v3.2.0 Feb 15, 2019
@dpshelio
Copy link

I've found out about this problem today. Very confusing the fact that one can access the whole matpltolib and other things like sys and time when imported everything from pyplot. It's very confusing seen scripts like:

from matplotlib.pyplot import *
...
matplotlib.pyplot.savefig('myimage.png')

I really support this change!

@timhoffm timhoffm modified the milestones: v3.2.0, v3.3.0 Aug 7, 2019
@timhoffm timhoffm modified the milestones: v3.3.0, v3.4.0 Apr 11, 2020
@timhoffm timhoffm marked this pull request as draft July 14, 2020 21:20
@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 21, 2021
@timhoffm timhoffm modified the milestones: v3.5.0, unassigned Jul 8, 2021
@story645 story645 modified the milestones: unassigned, needs sorting Oct 6, 2022
@tacaswell tacaswell modified the milestones: future releases, v3.7.0 Dec 5, 2022
@tacaswell tacaswell marked this pull request as ready for review December 5, 2022 23:33
@tacaswell
Copy link
Member

I took the liberty of rebasing this and think it is ready to review.

@tacaswell tacaswell modified the milestones: v3.7.0, v3.8.0 Dec 9, 2022
@tacaswell
Copy link
Member

I think this needs a behavior change note indicating that we restricted the namespace, but otherwise 👍

oscargus
oscargus approved these changes Jan 19, 2023
View reviewed changes
Copy link
Member

@oscargus oscargus left a comment

Choose a reason for hiding this comment

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

I think this makes sense. Will also help with #22149.

@timhoffm
Copy link
Member Author

Happy to see this moving. Please be aware that this may not yet be ready

Note: This is currently a proof of concept and requires additional work.

  • This affects pylab since pylab uses from matplotlib.pyplot import *, which is an unintended side-effect.
  • The hard-coded part of functions in __all__ needs some cleanup. Possible some are missing, while others are not neccesary.

I'm sorry I don't have the time to work on this. Anybody can take over the PR and (force) push.

@jklymak
Copy link
Member

jklymak commented Jan 24, 2023

Lets move to draft, but definitely anyone should feel free to put back...

@jklymak jklymak marked this pull request as draft January 24, 2023 17:19
@QuLogic QuLogic mentioned this pull request Mar 9, 2023
Open
5 tasks
@github-actions
Copy link

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label May 30, 2023
@timhoffm timhoffm added keep Items to be ignored by the “Stale” Github Action and removed status: inactive Marked by the “Stale” Github Action labels May 31, 2023
@timhoffm timhoffm force-pushed the pyplot-all branch 2 times, most recently from e9d8ff2 to e2c9eb9 Compare May 31, 2023 05:34
@timhoffm
Copy link
Member Author

Rebased, but #12743 (comment) still applies.

@timhoffm timhoffm modified the milestones: v3.8.0, future releases Aug 4, 2023
@ksunden ksunden mentioned this pull request Sep 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oscargus oscargus oscargus approved these changes

Assignees
No one assigned
Labels
keep Items to be ignored by the “Stale” Github Action status: needs rebase status: work in progress
Projects
None yet
Milestone
future releases
Development

Successfully merging this pull request may close these issues.
8 participants
@timhoffm @ImportanceOfBeingErnest @dpshelio @tacaswell @jklymak @oscargus @QuLogic @story645