Skip to content

Conversation

@ilakkmanoharan
Copy link
Contributor

PR summary

This PR fixes issue #30706.

Why this change is necessary

Passing a non-string value (e.g., np.array, int, None) to the orientation
parameter of Axes.grouped_bar() previously caused a misleading NumPy error:

ValueError: The truth value of an array with more than one element is ambiguous

This occurred because _api.check_in_list() attempted a membership check on a
non-scalar object, leading to an elementwise boolean comparison.

What problem it solves

This patch ensures that invalid (non-string) orientation values raise a clean,
user-facing ValueError instead of an ambiguous NumPy truth-value error.
This behavior now matches other Matplotlib functions (e.g., barh, stem)
that first validate enum arguments before list membership checks.

Implementation details

Added an early type guard in Axes.grouped_bar() before calling _api.check_in_list():

if not isinstance(orientation, str):
    raise ValueError(f"{orientation!r} is not a valid value for orientation")
_api.check_in_list(["vertical", "horizontal"], orientation=orientation)

Tests

Added a new test test_grouped_bar_orientation_invalid() to
lib/matplotlib/tests/test_axes.py verifying the following cases:

"invalid"
1
None
np.array([1, 2, 3])

Each now raises:

ValueError: '' is not a valid value for orientation

Result

Clean ValueError instead of ambiguous truth-value error
Consistent behavior with other Matplotlib APIs
All tests pass locally

PR checklist

closes #30706
new and changed code is tested
[N/A] Plotting related features are demonstrated in an example
[N/A] New Features and API Changes are noted with a directive and release note
[N/A] Documentation complies with general and docstring guidelines

Use pathlib.Path instead of matplotlib.path.Path in text.pyi
@rcomer
Copy link
Member

rcomer commented Oct 30, 2025

Something went wrong with your commit. Perhaps you used --amend by mistake.

@timhoffm
Copy link
Member

Thanks for the report. This is an issue within _check_in_list() and should be handled in there. - It applies to other uses of that function as well.

For a correct solution, you’ll also have to check whether _check_in_list() Always works on lists of strings -we mostly do, but I can’t say from the top of my head whether we have some other usage as well. If so the error handling would need to be more general than an isinstance check.

Consequently, the test should also be written for _check_in_list(). Writing tests for invalid inputs for every parameter of every method does not scale.

@rcomer
Copy link
Member

rcomer commented Oct 31, 2025

I did a quick search yesterday, and sometimes None is given as a valid value in the list. I did not check every instance.

@ilakkmanoharan
Copy link
Contributor Author

Thank you, @rcomer and @timhoffm, for the feedback — I’ll take a look and follow up.

@timhoffm
Copy link
Member

timhoffm commented Nov 1, 2025

@ilakkmanoharan Thanks for the bug report and the attempt to fix this. We've now done the proper fix ourselves in #30714. Please understand that core developer time is precious and in this case implementing the fix ourselves is much more efficient than guiding you towards the correct solution and subsequently reviewing it.

@timhoffm timhoffm closed this Nov 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Axes.grouped_bar() with non-string orientation (e.g., NumPy array) raises ambiguous truth-value error instead of clean ValueError

3 participants