Skip to content
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

boxplot does not take parameters into account #3042

Closed
Carreau opened this issue May 6, 2014 · 14 comments · Fixed by #3165
Closed

boxplot does not take parameters into account #3042

Carreau opened this issue May 6, 2014 · 14 comments · Fixed by #3165
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Milestone

Comments

@Carreau
Copy link
Contributor

Carreau commented May 6, 2014

And default style is changed.

Here is ho I used to have boxplot on matplotlib python 2
capture decran 2014-05-06 a 12 33 24

recent version on python 3 look different and seem not to take into account parameters passed to it.

capture decran 2014-05-06 a 12 32 54

Plot generated with

In [1]: %pylab
In [2]: boxplot(np.random.normal(size=500), sym='ro')
In [3]: matplotlib.__version__
Out[3]: '1.4.x'

(yes I know pylab is wrong)

git-desribe in mpl repo before a python3 setup install gives v1.3.1rc2-1231-gef462e4

My github foo did not show similar issues.

@tacaswell
Copy link
Member

@phobson can you take a look at this?

@tacaswell tacaswell added this to the v1.4.0 milestone May 6, 2014
@phobson
Copy link
Member

phobson commented May 6, 2014

Confirmed.

To get what @Carreau needs, you have to do:

%matplotlib inline
import matplotlib
import numpy as np
import matplotlib.pyplot as plt

fig, ax = plt.subplots()
bp = ax.boxplot(np.random.normal(size=500), flierprops=dict(marker='o', markerfacecolor='r', markeredgecolor='k'))

I have an idea for a fix. Should be simple.

@phobson
Copy link
Member

phobson commented May 6, 2014

@Carreau Just curious: are the changed defaults better, in your opinion. Aesthetics aside, should we have kept them the same?

@tacaswell
Copy link
Member

Defaults should not have changed (and the tests should have caught them if they did). Changing defaults is considered a major API break.

@Carreau
Copy link
Contributor Author

Carreau commented May 6, 2014

I am no designer, and I wouldn't even follow my own decision.
But I agree that change is API break.

But full recognizable Matplotlib theme would be nice.

@phobson
Copy link
Member

phobson commented May 7, 2014

Restoring the default symbolgy is easy, though I'm not sure I agree it's necessary.

I fully understand that the change was technically a bug and why that's the case. But the old style is just so...ugly. (@tacaswell the tests didn't catch it b/c i updated the test images to the new default style -- so yeah, mea culpa)

Reading the sym kwarg correctly will come in another PR.

@tacaswell
Copy link
Member

It needs to be fixed. I should have caught that you did that when I merged it.

The "it's ugly" position is one that has been taken before (and is what inspired (in part) the creation of prettyplot, seaborn, and ggplot). The problem is that if you change the defaults you break someones code and that even if we agreed to change the defaults, the only thing we all agree on is that we don't like the current defaults and it just turns into a flame war.

I think the solution is something along the lines of the rcparam stylesheets that @tonysyu has been working on and/or my rcparam refactor proposal ( #2637 ).

@phobson
Copy link
Member

phobson commented May 7, 2014

Yeah. I have witnessed and totally understand that. With the default style being the low-hanging fruit here, expect that PR first.

@Carreau
Copy link
Contributor Author

Carreau commented May 7, 2014

Agreed that its ugly but that we should not change default.

Related to #2637 I made something along as an experiement that rely on metaclass and make all artist configurable based on their class name. It also show my bad design skills.

@pelson
Copy link
Member

pelson commented Jun 18, 2014

The "it's ugly" position is one that has been taken before.

Changing defaults is considered a major API break.

Just for posterity, I'd like to put my oar in and say that avoiding all change is not only impossible, but also undesirable. If a package gets too scared to change defaults (where reasonable, infrequent, and easy to revert) is in its death throws (sorry numpy) which is not an aspiration I have for mpl. That said, the "ugly" argument can be fixed in other ways (style-sheets for instance) and we should perhaps be focusing on more of those.

In this case though, I agree with @tacaswell et al. - let's revert the default values. 👍

@Carreau
Copy link
Contributor Author

Carreau commented Jun 18, 2014

I supposed that's why version number have major-minor-patch :-)

@tacaswell
Copy link
Member

@Carreau Is the 0 vs 1 a big deal?

@tacaswell tacaswell reopened this Jul 4, 2014
@tacaswell
Copy link
Member

(sorry, wrong button)

@Carreau
Copy link
Contributor Author

Carreau commented Jul 4, 2014

Oh, I haven't even see the 0 vs one change on X axis. I tend to position the boxplot myself, so I don't care.

But I suppose people might be relying on it, so I would consider it API change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
4 participants