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

Add PEP8-compliant aliases to transAxes, transData, etc.#11051

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

Closed
anntzer wants to merge1 commit intomatplotlib:masterfromanntzer:pep8-transfoo

Conversation

anntzer
Copy link
Contributor

ax.transAxes / transData / etc. are perhaps one of the most user-facing
parts of mpl that are not pep8 compliant. Provide some PEP8-compliant
aliases for those (like myself) who are excessively (and unduly?)
bothered by that.

Note that (quite unusually for myself :-)) I amnot proposing to
deprecate the camelCase names.

PR Summary

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

@anntzeranntzer added this to thev3.0 milestoneApr 15, 2018
@QuLogic
Copy link
Member

Should we call them something likeaxes_transform instead of the other way?

@anntzer
Copy link
ContributorAuthor

Would be fine with that too.

@timhoffm
Copy link
Member

timhoffm commentedApr 16, 2018
edited
Loading

+1 for namingaxes_transform and similar.

In the conflict between backward compatibility and "There should be one-- and preferably only one --obvious way to do it." I propose the following:

  1. The new names should be the preferred choice.
  2. They should be the attributes, the old names should be made available via properties for backward compatibility.
  3. All internal code should use the new names.
  4. All examples should use the new names.
  5. The documentation of the old names should discourage their use and point at the alternatives.
  6. I agree that we should not yet deprecate the old names, but they should go in the long run (maybe 4.0ish). It's probably good to mark them as pending deprecation.

Not everything (in particular not 3, 4) has to be part of this PR, but we should discuss the further strategy here before merging this PR.

ax.transAxes / transData / etc. are perhaps one of the most user-facingparts of mpl that are not pep8 compliant.  Provide some PEP8-compliantaliases for those (like myself) who are excessively (and unduly?)bothered by that.Note that (quite unusually for myself :-)) I am **not** proposing todeprecate the camelCase names.
@anntzer
Copy link
ContributorAuthor

Renamed accordingly.
You get tosed through the codebase for switching to new names everywhere :-)

@timhoffm
Copy link
Member

Can do with 3-5.

Still, I would like to have at least 2. in this PR. Changing that later would basically revert this PR.

@anntzer
Copy link
ContributorAuthor

I actually think 2 & 3 should be done together (unless you can show that there's no performance issue, given how much we access these internally).
(FWIW I also think that deprecating the old names gets a bit close to "causing gratuitious code churn" (who'd have thought I'd once say this...).)

@timhoffm
Copy link
Member

Ok, for 2-3 I'd basically have to create a competing PR. But we should discuss and agree if we want 1-6 before starting further work.

@anntzer
Copy link
ContributorAuthor

I am +1 on 1-5 (feel free to open your own PR) and +0 on 6.

@efiring
Copy link
Member

I'm far from convinced that this renaming is a good idea--that its advantages (ability to avoid camelCase) outweighs its disadvantages (more code, more explanations required, two names for exactly the same thing).

jklymak and ImportanceOfBeingErnest reacted with thumbs up emoji

@tacaswelltacaswell modified the milestones:v3.0,v3.1Jul 15, 2018
@tacaswell
Copy link
Member

Moving to 3.1 as this will need more consideration.

@jklymak
Copy link
Member

Did we still want to try and discuss for 3.1?

@timhoffmtimhoffm modified the milestones:v3.1.0,v3.2.0Feb 14, 2019
@timhoffm
Copy link
Member

Moving further to 3.2 because this still needs some consideration and I don't see a dire need to rush this. Feel free to re-milestone.

anntzer reacted with thumbs up emoji

@timhoffmtimhoffm modified the milestones:v3.2.0,v3.3.0Aug 26, 2019
@QuLogicQuLogic modified the milestones:v3.3.0,v3.4.0May 2, 2020
@QuLogicQuLogic added the status: needs comment/discussionneeds consensus on next step labelMay 2, 2020
@anntzeranntzer deleted the pep8-transfoo branchSeptember 21, 2020 19:46
@anntzer
Copy link
ContributorAuthor

General agreement during call was to close this, but@timhoffm gets to reopen this if he wants :)

@timhoffm
Copy link
Member

Let's keep this closed for now. There are enough other battles to fight.

However, I reserve the right to revisit. 😄

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
status: needs comment/discussionneeds consensus on next step
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@anntzer@QuLogic@timhoffm@efiring@tacaswell@jklymak

[8]ページ先頭

©2009-2025 Movatter.jp