Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
timhoffm commentedApr 18, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
That would best be done with a new type of locator. Something like a |
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"). |
I guess I think if we are changing the API, it should be done at once? |
timhoffm commentedApr 18, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This PRIm 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 ideaThe 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
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. |
re: the alternative idea:
|
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 commentedApr 18, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
You could label only the two edges in this case, which can be a fallback for above *I tried to find such a plot on google, but didn't manage to. |
Fair point, but let's move this to another time? A new locator class means new tests to be written, too :) |
I opened#20025 to separately track further improvements. |
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° :/)
before:


after:
PR Summary
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).