Skip to content

DOC: Add note about locators at top of ticker docs #19002

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
Merged
merged 1 commit into from
Dec 17, 2020
Merged

DOC: Add note about locators at top of ticker docs #19002

merged 1 commit into from
Dec 17, 2020

Conversation

dopplershift
Copy link
Contributor

PR Summary

The note about not sharing Locator instances should be more prominent than hiding in the base class docs, which most users have no need to examine (since they'll use one of many subclasses 99% of the time).

I opted not to add anything to the text_intro.py tutorial, but I'm open to doing so if someone has suggestions on the right level of detail for that.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@anntzer
Copy link
Contributor

anntzer commented Nov 25, 2020

See also #13482 and #13439.

@jklymak
Copy link
Member

jklymak commented Nov 25, 2020

As discussed in gitter, I think we should fix this properly. OTOH, happy for this to go in when the build passes....

timhoffm
timhoffm reviewed Nov 25, 2020
View reviewed changes
@dopplershift
Copy link
Contributor Author

If we want this in the 3.3 docs, then I'm happy to update. If we're actually going to fix properly for 3.4, then there's no point in putting this in.

@jklymak
Copy link
Member

jklymak commented Nov 25, 2020

I think it'd be good to get this in because I have a feeling we won't be able to re-engineer Locators and Formatters by 3.4 (though who knows! Where there is a will there is a way).

@dopplershift dopplershift force-pushed the locator-note branch 2 times, most recently from 3f1ad82 to 0497934 Compare November 26, 2020 21:20
@dopplershift
DOC: Add note about locators at top of ticker docs
c1b0756 
The note about not sharing `Locator` instances should be more prominent
than hiding in the base class docs, which most users have no need to
examine.
@dopplershift
Copy link
Contributor Author

Should this be milestoned for 3.3-doc?

@tacaswell tacaswell added this to the v3.3-doc milestone Nov 28, 2020
@dopplershift
Copy link
Contributor Author

I don't think there's anything else this needs...

@QuLogic
Copy link
Member

QuLogic commented Dec 17, 2020

Seems fine to me.

@QuLogic QuLogic merged commit e780592 into matplotlib:master Dec 17, 2020
@lumberbot-app
Copy link

lumberbot-app bot commented Dec 17, 2020

Something went wrong ... Please have a look at my logs.

It seem that the branch you are trying to backport to does not exists.

@QuLogic
Copy link
Member

QuLogic commented Dec 17, 2020 •
edited
Loading

@meeseeksdev backport to v3.3.3-doc

1 similar comment
@QuLogic
Copy link
Member

QuLogic commented Dec 17, 2020

@meeseeksdev backport to v3.3.3-doc

@meeseeksmachine meeseeksmachine mentioned this pull request Dec 17, 2020
Merged
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Dec 17, 2020
@QuLogic @meeseeksmachine
Backport PR matplotlib#19002: DOC: Add note about locators at top of …
dd88dfe 
…ticker docs
@dopplershift dopplershift deleted the locator-note branch December 17, 2020 06:34
timhoffm added a commit that referenced this pull request Dec 17, 2020
@timhoffm
Merge pull request #19136 from meeseeksmachine/auto-backport-of-pr-19…
710ffd9 
…002-on-v3.3.3-doc

Backport PR #19002 on branch v3.3.3-doc (DOC: Add note about locators at top of ticker docs)
@gribbg
Copy link

gribbg commented Dec 23, 2020

Two other thoughts in this space:

  1. Can we update the documentation for set_major_locator and set_minor_locator to include Note: locator must be unique instance per axis?
  2. Any thoughts on making TickHelper.set_axis() issue a warning if self.axis is already set?

@timhoffm
Copy link
Member

  1. Basically 👍, though I would use the same wording as here.
  2. Generally 👍 on runtime warnings for non-intended usage. TickHelper is also the base class for Formatters, and AFAIK the restriction does not apply to them. So the warning would have to be implemented in the Locator class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic approved these changes

@timhoffm timhoffm Awaiting requested review from timhoffm

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
v3.3-doc
Development

Successfully merging this pull request may close these issues.
7 participants
@dopplershift @anntzer @jklymak @QuLogic @gribbg @timhoffm @tacaswell