Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Use FixedFormatter only with FixedLocator#15507
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
tacaswell commentedOct 25, 2019
Seems reasonable, but fails 15 tests! |
4f9a9fe to332cb4aCompareUh oh!
There was an error while loading.Please reload this page.
jklymak commentedNov 1, 2019
|
Uh oh!
There was an error while loading.Please reload this page.
578236a to29b63c7CompareUh oh!
There was an error while loading.Please reload this page.
anntzer left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
just one nit
Uh oh!
There was an error while loading.Please reload this page.
actually a real (minor) thing to fix on the warning too
8481e72 toa473ba1Comparetimhoffm commentedDec 30, 2019
Ping@anntzer warning fixed. |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
This PR prevents users from stumbling into#7122,#8779,#15501.
It does not technically fix them,
FixedFormatterhas a one-off bug with locators other thanFixedLocator. But setting fixed labels at unknown positions is anyway not a good idea. So even if the one-off bug was fixedFixedFormattershould only used withFixedLocator.FixedFormattershould not be used without aFixedLocator(along the hierarchy of tick label setter methods).set_major/minor_formatter()if aFixedFormatteris set without aFixedLocator. This may be debatable.One could set the formatter first and the locator afterwards, which would technically yield the correct result. However, I feel that setting the position first and the labels second is the natural way, and is actually done throughout the library.
No alternative: Since Formatters don't know about Locators
FixedFormatteritself cannot check at runtime and we have to resort to this somewhat clumsy approach.And I strongly want some feedback on this potential problem from running code, because nobody reads the docs 😄.