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

Fix default theta tick locations for non-full-circle polar plots.#20012

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
jklymak merged 1 commit intomatplotlib:masterfromanntzer:pt
Apr 20, 2021

Conversation

anntzer
Copy link
Contributor

Currently, non-full-circle polar plots may (depending on the axes size)
put theta ticks at positions that make sense for linear plots, but are a
bit weird for polar plots (e.g. 50° or 25°). Instead, prefer "standard"
angles (90°, 45°, 30°, 15°, 10°). For really small axes, there's still
a problem because a single tick may end up at 150°, but fixing the
machinery to allow 15° while rejecting 150° would be trickier.
(Likewise, really thin slivers likely want ticks at 0.5° steps rather
than 0.45° :/)

frompylabimport*foraxingcf().subplot_mosaic("AAAABBC",subplot_kw={"projection":"polar"}).values():ax.set_thetalim(0,np.pi)

before:
old
after:
new

PR Summary

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

@timhoffm
Copy link
Member

timhoffm commentedApr 18, 2021
edited
Loading

but fixing the
machinery to allow 15° while rejecting 150° would be trickier.

That would best be done with a new type of locator. Something like aChoiceLocator that can only select from given sets of values.

@anntzer
Copy link
ContributorAuthor

I know, I was just hoping that this PR can be considered a stepwise improvement and defer ChoiceLocator to later (well, it's more complicated that ChoiceLocator, it's really "below a given scale, behave as MaxNLocator, above it limit yourself to some fixed values").

@jklymak
Copy link
Member

I guess I think if we are changing the API, it should be done at once?

@timhoffm
Copy link
Member

timhoffm commentedApr 18, 2021
edited
Loading

This PR

Im ok with an incremental improvement. I doubt that anybody relies on the current behavior as something they explicitly want. In particular, the second time changing would only affect the very right example. If somebody is ok with 100°, it means they don't realy care, and 150° and later 90° should be fine for them.

If we don't want to invest addtional time in this now it's ok to get the more common cases right at the potential cost of changing an edge-case twice.


Alternative idea

The theta-scale is much simpler than other scales, because it's finite and we can thus hard-wire all reasonable solutions.

Maybe I'm underestimating this, but I would have assumed that we can define

ChoiceLocator([    [0, 30, 60, 90, 120, 150, 180, 210, 240, 270, 300, 330],    [0, 45, 90, 135, 180, 225, 270, 315],    [0, 60, 120, 180, 240, 300],    [0, 90, 180, 270],])

which can choose one of the given lists to draw tick locations from. Your first plot would choose the first list, the second may choose the second list, and the third might choose the fourth list. The only thing I'm not sure about how is how the locator knows about the available space and can thus choose more or less values, but apparently the current locator does that already, so it should be possible.

@anntzer
Copy link
ContributorAuthor

re: the alternative idea:

  1. The locator knows the available space viaAxis.get_tick_space... which actually likely needs a reimplementation both for theta and r (see e.g. the overlaps in the r ticks for the small axes), but that's a separate issue.
  2. The problem would be for thin slivers (plotted on really large axes), e.g. from 0° to 10°, where you do want to fall back to the "normal" (MaxNLocator) approach. Again, that's certainly fixable, just needs a bit more work thanjust setting the steps on the current MaxNLocator...

Currently, non-full-circle polar plots may (depending on the axes size)put theta ticks at positions that make sense for linear plots, but are abit weird for polar plots (e.g. 50° or 25°).  Instead, prefer "standard"angles (90°, 45°, 30°, 15°, 10°).  For really small axes, there's stilla problem because a single tick may end up at 150°, but fixing themachinery to allow 15° while rejecting 150° would be trickier.(Likewise, really thin slivers likely want ticks at 0.5° steps ratherthan 0.45° :/)
@timhoffm
Copy link
Member

timhoffm commentedApr 18, 2021
edited
Loading

The problem would be for thin slivers (plotted on really large axes)

You could label only the two edges in this case, which can be a fallback for aboveChoiceLocator if the theta-range is too small. That should begood enough for a default. IMHO this is really an edge-edge case*, and we don't have to over-think it.

*I tried to find such a plot on google, but didn't manage to.

@anntzer
Copy link
ContributorAuthor

Fair point, but let's move this to another time? A new locator class means new tests to be written, too :)

timhoffm reacted with thumbs up emojitimhoffm reacted with laugh emoji

@anntzer
Copy link
ContributorAuthor

I opened#20025 to separately track further improvements.

@jklymakjklymak merged commit5c8cf78 intomatplotlib:masterApr 20, 2021
@anntzeranntzer deleted the pt branchApril 20, 2021 14:52
@QuLogicQuLogic added this to thev3.5.0 milestoneApr 20, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jklymakjklymakjklymak approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.5.0
Development

Successfully merging this pull request may close these issues.

4 participants
@anntzer@timhoffm@jklymak@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp