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

Revert renaming labels to tick_labels in boxplot_stats() #27916

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

timhoffm
Copy link
Member

This is up for debate: I'm only +0.2 here.

The renaming was done in #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 (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 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, 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().

@timhoffm
Copy link
Member Author

Still need to revert the parameters in the tests. But before doing that, I'd like to get feedback on the proposal.

@tacaswell tacaswell added this to the v3.9.0 milestone Mar 14, 2024
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Mar 14, 2024
@tacaswell
Copy link
Member

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 in matplotlib#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, 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()`.
Copy link
Member

@dstansby dstansby left a 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, since boxplot_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).

@dstansby
Copy link
Member

Anyone can merge if appveyor CI passes.

@timhoffm timhoffm merged commit 4b0eee4 into matplotlib:main Mar 15, 2024
42 checks passed
@timhoffm timhoffm deleted the bxpstats branch March 15, 2024 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation: examples files in galleries/examples Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants