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

Update handling of sequence labels for plot #27767

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Feb 10, 2024

PR summary

Closes #27762.

  • plot(x, y, label=['foo']) where y is 1D now results in the legend label "foo" instead of "['foo']" (immediate change).
  • plot(x, y, label=sequence) where y is 1D and sequence is not of length 1 is deprecated to error in future.

PR checklist

@rcomer

This comment was marked as outdated.

@rcomer

This comment was marked as outdated.

@rcomer rcomer marked this pull request as ready for review February 10, 2024 18:51
timhoffm

This comment was marked as outdated.

@rcomer

This comment was marked as outdated.

@timhoffm

This comment was marked as outdated.

@rcomer

This comment was marked as outdated.

@timhoffm

This comment was marked as outdated.

@timhoffm
Copy link
Member

@rcomer I've introduced #27780, which should resolve the boxplot issues you've had with this PR. I suggest to wait until that gets merged, then rebase on it and remove all the boxplot specifics here.

@rcomer
Copy link
Member Author

rcomer commented Feb 15, 2024

Hid a bunch of comments because they discuss issues with boxplot that are now being handled elsewhere. This change no longer touches boxplot.


if cbook.is_scalar_or_string(label):
labels = [label] * n_datasets
else:
if len(label) != n_datasets:
Copy link
Member

@timhoffm timhoffm Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional but inverting the if branches is more readable:

Suggested change
if len(label) != n_datasets:
if len(label) == n_datasets:
  • you remove the negation in the condition
  • And the short and good-case branch comes first

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little sleepy but I think this if doesn't have an else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think I get it. I can have a look this evening (on the wrong computer at the moment).

@ksunden
Copy link
Member

ksunden commented Feb 15, 2024

Anyone can merge on green CI

@QuLogic QuLogic merged commit af80c90 into matplotlib:main Feb 15, 2024
42 checks passed
@rcomer rcomer deleted the plot-single-legend branch February 15, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Inconsistent treatment of list of labels in plot when the input is a dataframe
4 participants