The note about not sharingLocator 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 thetext_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 (andpytest passes).
IsFlake 8 compliant (runflake8 on changed files to check).
New features are documented, with examples if plot related.
Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
Conforms to Matplotlib style conventions (installflake8-docstrings and runflake8 --docstring-convention=all).
New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

dopplershift added the Documentation label

Nov 25, 2020

Copy link

Contributor

anntzer commentedNov 25, 2020

jklymak commentedNov 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 reviewed

Nov 25, 2020

View reviewed changes

lib/matplotlib/ticker.py OutdatedShow resolvedHide resolved

Copy link

ContributorAuthor

dopplershift commentedNov 25, 2020

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.

Copy link

Member

jklymak commentedNov 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 force-pushed thelocator-note branch 2 times, most recently from3f1ad82 to0497934Compare

November 26, 2020 21:20

DOC: Add note about locators at top of ticker docs

c1b0756

The note about not sharing `Locator` instances should be more prominentthan hiding in the base class docs, which most users have no need toexamine.

dopplershift force-pushed thelocator-note branch from0497934 toc1b0756Compare

November 28, 2020 16:10

Copy link

ContributorAuthor

dopplershift commentedNov 28, 2020

Should this be milestoned for 3.3-doc?

tacaswell added this to thev3.3-doc milestone

Nov 28, 2020

dopplershift requested a review fromtimhoffm

December 16, 2020 23:05

Copy link

ContributorAuthor

dopplershift commentedDec 16, 2020

I don'tthink there's anything else this needs...

QuLogic approved these changes

Dec 17, 2020

View reviewed changes

Copy link

Member

QuLogic commentedDec 17, 2020

Seems fine to me.

QuLogic merged commite780592 intomatplotlib:master

Dec 17, 2020

Copy link

lumberbot-appbot commentedDec 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.

Copy link

Member

QuLogic commentedDec 17, 2020•
edited
Loading

@meeseeksdev backport to v3.3.3-doc

1 similar comment

Copy link

Member

QuLogic commentedDec 17, 2020

@meeseeksdev backport to v3.3.3-doc

meeseeksmachine mentioned this pull request

Dec 17, 2020

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

Merged

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request

Dec 17, 2020

Backport PRmatplotlib#19002: DOC: Add note about locators at top of …

dd88dfe

…ticker docs

dopplershift deleted the locator-note branch

December 17, 2020 06:34

timhoffm added a commit that referenced this pull request

Dec 17, 2020

Merge pull request#19136from meeseeksmachine/auto-backport-of-pr-19…

710ffd9

…002-on-v3.3.3-docBackport PR#19002 on branch v3.3.3-doc (DOC: Add note about locators at top of ticker docs)

Copy link

gribbg commentedDec 23, 2020

Two other thoughts in this space:

Can we update the documentation forset_major_locator andset_minor_locator to includeNote: locator must be unique instance per axis?
Any thoughts on makingTickHelper.set_axis() issue a warning ifself.axis is already set?

Copy link

Member

timhoffm commentedDec 23, 2020

Basically 👍, though I would use the same wording as here.
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 theLocator class.