Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Logit scale nonsingular#14928
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
Logit scale nonsingular#14928
Uh oh!
There was an error while loading.Please reload this page.
Conversation
693ecee to966cca4CompareUh oh!
There was an error while loading.Please reload this page.
966cca4 to787f6b8CompareUh oh!
There was an error while loading.Please reload this page.
lib/matplotlib/ticker.py Outdated
| standard_minpos = 1e-7 | ||
| initial_range = (standard_minpos, 1 - standard_minpos) | ||
| swap_vlims = False |
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.
As a state flag, I would rather name thisvlims_swapped.
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.
I agree. But I take the logic (and the name for variables) fromLogLocator.
What is the best?
- use
vlims_swappedinLogitLocator, - use
swap_vlimsinLogitLocatorfor consistency withLogLocator, - use
vlims_swappedinLogitLocator, and renameswap_vlimstovlims_swappedinLogLocatorfor consistency,
I dislike the third one (as it can introduce conflicts with other branches for no real reason). I have no opinion between the two firsts.
Thanks@jb-leger ! We all mostly fix bugs that we did not write ;) |
Sorry, in#14624 we decided to revert the behavior of nonsingular to always order its return value in increasing order (it's a bit of a mess :( see linked discussions); it means that for this PR the swap_vlims dance should go away. |
Due to changes in other PRs, need to not swap the order
ok, I am apparently deeply confused about this sorting stuff.... |
58d39e2 todfa8367CompareSorry, I have a lot of administrative work these days, I did'nt had the time to interact here and to do the requested changes. That's perfect@tacaswell, I don't mind at all that you push on this branch. |
Inmatplotlib#14624 we revertedchanging the order of the limits to match the input order. Doing thesame here for consistency.Sort the limits before comparing. We want to make sure that it workswith the values reversed, but do not expect non-singular to preservethe order.
PR Summary
This PR solves issues#14743.
This issues was present from the
nonsingularmethod of theLogitLocator. In the PR#14512 I rewrite most of this locator, but I keep the nonsingular method from the old code. I was not the author of this bug. I fixed it because I know this very small part of the matplotlib code.Also, I added some tests.
PR Checklist