-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Validate steps input to MaxNLocator
#7578
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Bisect points to
|
First, evidently cartopy was using a
Second, I think the difference in behavior is an improvement, in that it does a better job of getting close to the target (maximum) number of bins (tick intervals within the limits), given the available steps. I will add a note to the documentation, but I don't consider the new behavior to be a bug and I will not try to restore the old behavior. This will be one of the relatively few areas where behavior in 2.0, even with classic settings, is not exactly the same as in 1.5.x. Evidently I also need to add validation of the steps kwarg. |
I'm moving this to milestone 2.1 |
Maybe the validation part, but this issue is a missing documentation problem of a change in 2.0; it should go in 2.0, or at most 2.0.1. |
I am fine with the documentation being into 2.0, but we need to be extremely careful about not tagging too much issue into 2.0, else we will never release 2.0. There are way too many issues and PR being tagged 2.0.1. |
The 2.0.1 milestone is irrelevant for the purposes of releasing 2.0; it's a nice-to-have list, but not that important. For 2.0, I bumped everything that's not documentation or imminent (I assume @tacaswell wants those last two features in) or a regression. |
Wait wait, I forgot one very important thing; tests were run in classic style to minimize differences with older releases. If I compare classic mode with 1.5.3, then using the valid So I must apologize for all the milestone flipping, but since this is only a problem when given invalid input, I guess it's not that important to document in 2.0 specifically. We only really need the validation part, which can go into 2.1. |
MaxNLocator
in v2.0.0rc1MaxNLocator
I have the validation plus a bugfix (an inconsistency with "integer=True") ready to go, and I will add a doc note soon and then make a PR. With the reimplementation of the inner algorithm in MaxNLocator I think there will be cases where the result differs, so it would be good to mention this. I don't see any problem with including it in 2.0. With luck I will have the PR ready tonight. |
NOTE The following was actually incorrect as the
steps
input was invalid; thus this ticket has been modified slightly to note the fact thatsteps
should be validated in some manner.Differences in
MaxNLocator
between 2.0.0 and 1.5.3 may be minimized by ensuringsteps
is between 1 and 10 and the classic style is used.I ran the Cartopy test suite against v2.0.0rc1 and there appear to be some changes in the result from
MaxNLocator
. The default style change page only references changes toAutoLocator
and the API changes page mentionsLinearScaleLocator
. I don't see any obvious changes in the tests forMaxNLocator
, so I guess this is a bug, or needs to be better documented.Here is the result on
1.5.3
:and here is the result with
v2.0.0rc1
that demonstrates the issue i.e a small piece of code which reproduces the issue
and can be run with out any other (or as few as possible) external dependencies.
note where it used to work.
The text was updated successfully, but these errors were encountered: