Skip to content

Fix passing singleton sequence-type styles to hist #29719

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

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Mar 8, 2025

PR summary

Fixes #29717. Use of np.atleast_1d here is not appropriate for style properties expressed as tuples or array-likes. For colours we can just use to_rgba_array. For linestyles I factored out the logic used to set one or more linestyles in Collection.

We are already using to_rgba_array if colors is passed:

else:
colors = mcolors.to_rgba_array(color)

PR checklist

@rcomer rcomer added the PR: bugfix Pull requests that fix identified bugs label Mar 8, 2025
@rcomer
Copy link
Member Author

rcomer commented Mar 8, 2025

OK so we can't just use to_rgba_array if the colour is "none", which doesn't happen in the tests but does in the docs...

@rcomer rcomer closed this Mar 8, 2025
@rcomer rcomer deleted the hist-styles branch March 8, 2025 18:45
@rcomer rcomer restored the hist-styles branch March 8, 2025 18:46
@rcomer rcomer reopened this Mar 8, 2025
@story645
Copy link
Member

story645 commented Mar 9, 2025

OK so we can't just use to_rgba_array if the colour is "none",

Is this related to #28475?

@rcomer
Copy link
Member Author

rcomer commented Mar 9, 2025

OK so we can't just use to_rgba_array if the colour is "none",

Is this related to #28475?

Yes exactly. The array has zero size, so you get a StopIteration error when you try to call next.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I have been pondering for a time whether we should have an internal helper to loop over possibly list-type properties. This should take care of all the technical details like single value vs. list of values, properly handling sequence-like single values, optionally length checks (we currently don't have a clear policy whether we request the same size, or whether we are permissive and continuously cycle as long as needed, which may be handy in some cases but could hide a bug in others).

@rcomer rcomer added this to the v3.10.2 milestone Mar 11, 2025
@story645 story645 merged commit 4b68626 into matplotlib:main Mar 12, 2025
55 of 65 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Mar 12, 2025
story645 added a commit that referenced this pull request Mar 12, 2025
…719-on-v3.10.x

Backport PR #29719 on branch v3.10.x (Fix passing singleton sequence-type styles to hist)
@rcomer rcomer deleted the hist-styles branch March 12, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: bugfix Pull requests that fix identified bugs topic: collections and mappables
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Using a linestyle tuple with a histogram crashes with matplotlib 3.10
3 participants