Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Revert renaming labels to tick_labels in boxplot_stats()#27916
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Still need to revert the parameters in the tests. But before doing that, I'd like to get feedback on the proposal. |
This is reverting a deprecation so it needs to get done before it goes out. |
This is up for debate: I'm only +0.2 here.The renaming was done inmatplotlib#27901, which renamed the parameter `labels`to `tick_labels` for `boxplot()` and `bxp()`.One can take two views here:- If `boxplot_stats()` is specifically for the input of `bxp()`, one can justify the renaming as being consistent with the `bxp()` signature. Note however, that the returned dict still contains the key "label" for back-compatibility. So that brings us an inconsistency between the parameter name and the returned dict key.- One can alternatively view `boxplot_stats()` as a generic function to define box parameters. Here, we'd only have a general `label` for the boy and no information that this should be used as tick label.If we could make a clean transition and also rename the dict key, Iwould tend to go with the first view. But the inevitable inconsistencyof the fist view let's me sway towards the second view, so that withthe revert, we've effectively not touched `boxplot_stats()`.
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.
Good catch - I'm +1 to changing this back, sinceboxplot_stats() could be used independently of any plotting, and therefore without the context of having any tick labels (even though that's what the labels happen to be used for).
Anyone can merge if appveyor CI passes. |
This is up for debate: I'm only +0.2 here.
The renaming was done in#27901, which renamed the parameter
labelstotick_labelsforboxplot()andbxp().One can take two views here:
If
boxplot_stats()is specifically for the input ofbxp(), one can justify the renaming as being consistent with thebxp()signature. Note however, that the returned dict still contains the key "label" for back-compatibility (There's no easy migration path for that). So, this brings us an inconsistency between the parameter name and the returned dict key.One can alternatively view
boxplot_stats()as a generic function to define box parameters. Here, we'd only have a generallabelfor the boy and no information that this should be used as tick label.If we could make a clean transition and also rename the dict key, I would tend to go with the first view. But the inevitable inconsistency of the fist view let's me sway towards the second view, so that with the revert, we've effectively not touched
boxplot_stats().