Logit scale nonsingular #14928
Logit scale nonsingular #14928
Conversation
693ecee to
966cca4
Compare
966cca4 to
787f6b8
Compare
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.
As a state flag, I would rather name this vlims_swapped.
There was a problem hiding this comment.
I agree. But I take the logic (and the name for variables) from LogLocator.
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 to
dfa8367
Compare
|
Sorry, 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. |
In matplotlib#14624 we reverted changing the order of the limits to match the input order. Doing the same here for consistency. Sort the limits before comparing. We want to make sure that it works with the values reversed, but do not expect non-singular to preserve the 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