set_params should blacklist nbins as it will be ignored.

set_params should blacklist nbins as it will be ignored.

You mean, raise an exception if it's there?

Yes

On Tue, Dec 1, 2015 at 6:22 PM Michael Droettboom notifications@github.com
wrote:

set_params should blacklist nbins as it will be ignored.

You mean, raise an exception if it's there?

—
Reply to this email directly or view it on GitHub
#5588 (comment)
.

Done.

Does this mean that the default will now always be AutoLocator? And that a user will no longer be able to use locator_params(nbins=4) to adjust it? If so, I think this is a mistake. Letting the initial selection of nbins be done by an algorithm is good; but making it impossible to override this with a kwarg is not good. Or have I misunderstood?

@efiring: That's a good point. I've simplified this so that nbins == None can be used to indicate auto mode. This is the default for "new style", whereas classic mode will use the old default of 9. This actually significantly reduces the patch size as well.

It might be better to use the string 'auto' as the default? It is clearer
we will be doing something magic.

On Wed, Dec 2, 2015, 07:54 Michael Droettboom notifications@github.com
wrote:

@efiring https://github.com/efiring: That's a good point. I've
simplified this so that nbins == None can be used to indicate auto mode.
This is the default for "new style", whereas classic mode will use the old
default of 9. This actually significantly reduces the patch size as well.

—
Reply to this email directly or view it on GitHub
#5588 (comment)
.

It might be better to use the string 'auto' as the default? It is clearer we will be doing something magic.

Sure.

Right, "auto" is better for meaning automatic, meanwhile, None can continue
to mean "do the default thing".

On Wed, Dec 2, 2015 at 8:24 AM, Michael Droettboom <notifications@github.com

wrote:

It might be better to use the string 'auto' as the default? It is clearer
we will be doing something magic.

Sure.

—
Reply to this email directly or view it on GitHub
#5588 (comment)
.

Good. Now, I am wondering whether there is any point in making the additional AutoLocator class; wouldn't it be simpler to fold the small modification of __init__ into MaxNLocator itself? Then the selection of one functionality or the other is entirely a matter of how the nbins kwarg is set.

Good. Now, I am wondering whether there is any point in making the additional AutoLocator class; wouldn't it be simpler to fold the small modification of init into MaxNLocator itself? Then the selection of one functionality or the other is entirely a matter of how the nbins kwarg is set.

AutoLocator isn't new -- it's been there forever. It sets up reasonable defaults for MaxNLocator. I agree it doesn't have much reason to exist, but removing would theoretically break backward compatibility.

Just rebased. I think this is good-to-go once Travis passes.

And... it's green (on Travis)

@mdboom, I suspect some corner cases will turn up and tweaks might be needed, but I think it makes sense to merge this so that it will get more exercise.

Agreed. Thanks.

This was merged to master, but the merge is in the v2.0.x branch. I think something fishy went on around b7c7c5e but I am not sure.

backported as 7488da6

@mdboom, I finally took a look to see what this does on master, with a minimal plot plot([1,2,3]) on OS-X, using the qt4agg backend, and I think there is a problem: it is coming up with 11 ticks. Yes, the labels all fit, but it looks bad--very cluttered. I suggest that the default max number should be much smaller, more similar to what it used to be.

@efiring: You're suggesting a hard maximum? Just adding more space is unlikely to be a good fix, since in cases with many small axes you do want to allow the ticks to get reasonably close together.

Yes, a hard maximum is what I had in mind. Then the behavior would not change for the default single-axes-per-figure case, but the multiple axes case would still be supported.

Of course that hard maximum could be controllable with yet another rcParam if necessary. We didn't have one for that before, though.

I'm fine with just setting the max to the old hard-coded value (which appears to be 9) and we can always add an rcParam later if people want more control.