Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Cleanup imports.#9938

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

Merged
efiring merged 2 commits intomatplotlib:masterfromanntzer:more-unused-imports
Dec 29, 2017
Merged

Conversation

anntzer
Copy link
Contributor

PR Summary

Same as#9929, but for the lib itself: remove unused imports and sort/group some remaining imports.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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

@anntzeranntzerforce-pushed themore-unused-imports branch 6 times, most recently from25e008a toec9cc33CompareDecember 5, 2017 22:45
@tacaswell
Copy link
Member

Are there any benefits to this other than a bit of organization?

@tacaswelltacaswell added this to thev2.2 milestoneDec 5, 2017
@anntzer
Copy link
ContributorAuthor

Cutting down on pyflakes output...

@dopplershift
Copy link
Contributor

Removing unused imports could improve matplotlib's import time and memory usage. I'm not saying necessarily significantly, but I'll take what I can get for free. 😁

@jklymak
Copy link
Member

PEP 8 doesn't likeanimation.py

@tacaswell
Copy link
Member

It only helps if they are thing that would not be imported other wise. If things are already in sys.modules the import is basically free.

@anntzer
Copy link
ContributorAuthor

Note that I never claimed this was for performance :p
PEP8 should be appeased now...

@tacaswell
Copy link
Member

I think we explicitly do not check this bit of pep8, if we are going to do this travis should enforce it.

The dark side of 'discover-the-api-via-tab-complete' is that users will have found and used some of those un-used imports so I expect this to break some user code. It should get some level of mention in the API changes docs.

@anntzer
Copy link
ContributorAuthor

Not saying that this PR fixes pep8; rather, pep8 was unhappy because it wanted conditional imports (vs Python version) to be at the bottom of the import list.
Added api changes note.

@dopplershift
Copy link
Contributor

@tacaswell I agree...that's why I said "could". 😁 Also, the unused import adds to the module's globals dict (not to mention takes a few instructions to execute), so that's technically more memory usage (and CPU). I'm not saying it's significant, but it's a "free" fix to not ask for things you don't use.

@anntzer
Copy link
ContributorAuthor

grouped some more imports (in a separate commit)

@efiringefiring merged commit64874e5 intomatplotlib:masterDec 29, 2017
@anntzeranntzer deleted the more-unused-imports branchDecember 29, 2017 02:48
@QuLogicQuLogic modified the milestones:needs sorting,v2.2.0Feb 12, 2018
@anntzeranntzer mentioned this pull requestFeb 17, 2018
6 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.2.0
Development

Successfully merging this pull request may close these issues.

7 participants
@anntzer@tacaswell@dopplershift@jklymak@rhysparry@efiring@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp