8000 Add PEP8-compliant aliases to transAxes, transData, etc. by anntzer · Pull Request #11051 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

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.

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
Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented Apr 15, 2018

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 am not 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

@anntzer anntzer added this to the v3.0 milestone Apr 15, 2018
@QuLogic
Copy link
Member
QuLogic commented Apr 15, 2018

Should we call them something like axes_transform instead of the other way?

@anntzer
Copy link
Contributor Author
anntzer commented Apr 15, 2018

Would be fine with that too.

@timhoffm
Copy link
Member
timhoffm commented Apr 16, 2018 •
edited
Loading

+1 for naming axes_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.

@anntzer
Add PEP8-compliant aliases to transAxes, transData, etc.
4a8796b 
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 am **not** proposing to
deprecate the camelCase names.
@anntzer
Copy link
Contributor Author
anntzer commented Apr 16, 2018

Renamed accordingly.
You get to sed 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
Contributor Author
anntzer commented Apr 16, 2018

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
Contributor Author
anntzer commented Apr 16, 2018

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

@efiring
Copy link
Member
efiring commented Apr 16, 2018

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).

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

Moving to 3.1 as this will need more consideration.

@jklymak
Copy link
Member
jklymak commented Feb 11, 2019

Did we still want to try and discuss for 3.1?

@timhoffm timhoffm modified the milestones: v3.1.0, v3.2.0 Feb 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.

@timhoffm timhoffm modified the milestones: v3.2.0, v3.3.0 Aug 26, 2019
@QuLogic QuLogic modified the milestones: v3.3.0, v3.4.0 May 2, 2020
@QuLogic QuLogic added the status: needs comment/discussion needs consensus on next step label May 2, 2020
@anntzer anntzer closed this Sep 21, 2020
@anntzer anntzer deleted the pep8-transfoo branch September 21, 2020 19:46
@anntzer
Copy link
Contributor Author
anntzer commented Sep 21, 2020

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

@timhoffm
Copy link
Member
timhoffm commented Nov 2, 2020

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

However, I reserve the right to revisit. 😄

@QuLogic QuLogic removed this from the v3.4.0 milestone Mar 16, 2021
@tacaswell tacaswell mentioned this pull request May 25, 2023
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
status: needs comment/discussion needs 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
0