FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Raise ValueError for non-string orientation in Axes.grouped_bar (#30706) by ilakkmanoharan · Pull Request #30707 · matplotlib/matplotlib · GitHub

Raise ValueError for non-string orientation in Axes.grouped_bar (#30706) - #30707

Closed
ilakkmanoharan wants to merge 1 commit into
matplotlib:mainfrom
ilakkmanoharan:fix/grouped-bar-orientation-validation
Closed

Raise ValueError for non-string orientation in Axes.grouped_bar (#30706)#30707
ilakkmanoharan wants to merge 1 commit into
matplotlib:mainfrom
ilakkmanoharan:fix/grouped-bar-orientation-validation

Conversation

Copy link
Copy Markdown
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 commented Oct 30, 2025
edited
Loading

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
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 commented Oct 31, 2025

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

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

timhoffm commented Nov 1, 2025

Copy link
Copy Markdown
Member

@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 closed this Nov 1, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL