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

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

Merged
anntzer merged 1 commit intomatplotlib:masterfromtimhoffm:fixed-formatter
Dec 30, 2019

Conversation

@timhoffm
Copy link
Member

@timhoffmtimhoffm commentedOct 24, 2019
edited
Loading

PR Summary

This PR prevents users from stumbling into#7122,#8779,#15501.
It does not technically fix them,FixedFormatter has 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 fixedFixedFormatter should only used withFixedLocator.

  • This PR mostly documents thatFixedFormatter should not be used without aFixedLocator (along the hierarchy of tick label setter methods).
  • Additionally it warns on the lowest levelset_major/minor_formatter() if aFixedFormatter is 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 LocatorsFixedFormatter itself 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 😄.
  • One could consider captureing and re-emitting warnings on the higher levels with more appropriate messages, but I felt this was too much fuss.

@tacaswelltacaswell added this to thev3.3.0 milestoneOct 25, 2019
@tacaswell
Copy link
Member

Seems reasonable, but fails 15 tests!

@jklymak
Copy link
Member

TypeError: _warn_external() got an unexpected keyword argument 'stacklevel' 😉

timhoffm reacted with eyes emoji

@timhoffmtimhoffmforce-pushed thefixed-formatter branch 2 times, most recently from578236a to29b63c7CompareNovember 1, 2019 17:39
anntzer
anntzer previously approved these changesDec 2, 2019
Copy link
Contributor

@anntzeranntzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

just one nit

@anntzeranntzer dismissed theirstale reviewDecember 2, 2019 18:30

actually a real (minor) thing to fix on the warning too

@timhoffmtimhoffmforce-pushed thefixed-formatter branch 2 times, most recently from8481e72 toa473ba1CompareDecember 3, 2019 05:17
@timhoffm
Copy link
MemberAuthor

Ping@anntzer warning fixed.

@anntzeranntzer merged commit642b034 intomatplotlib:masterDec 30, 2019
@timhoffmtimhoffm deleted the fixed-formatter branchDecember 30, 2019 17:15
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@anntzeranntzeranntzer approved these changes

@jklymakjklymakjklymak approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

v3.3.0

Development

Successfully merging this pull request may close these issues.

4 participants

@timhoffm@tacaswell@jklymak@anntzer

[8]ページ先頭

©2009-2025 Movatter.jp