-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Enh/Add hatch pattern support to Axes.grouped_bar #30726
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
Changes from 3 commits
cd9ddcd
6e3a7c8
dc94475
b402730
90f0a22
0fad966
1eb63f9
960914c
adf2717
a8c8705
d2f1943
afb64cb
485ef67
9fffd16
af21213
541bf68
324d578
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3050,7 +3050,7 @@ def broken_barh(self, xranges, yrange, align="bottom", **kwargs): | |
| @_docstring.interpd | ||
| def grouped_bar(self, heights, *, positions=None, group_spacing=1.5, bar_spacing=0, | ||
| tick_labels=None, labels=None, orientation="vertical", colors=None, | ||
| **kwargs): | ||
| hatch=None,**kwargs): | ||
| """ | ||
| Make a grouped bar plot. | ||
|
|
||
|
|
@@ -3190,6 +3190,21 @@ def grouped_bar(self, heights, *, positions=None, group_spacing=1.5, bar_spacing | |
|
|
||
| If not specified, the colors from the Axes property cycle will be used. | ||
|
|
||
| hatch : sequence of str or None, optional | ||
| Hatch pattern(s) to apply per dataset. | ||
|
|
||
| - If ``None`` (default), no hatching is applied. | ||
| - If a sequence of strings is provided (e.g., ``['//', 'xx', '..']``), | ||
| the patterns are cycled across datasets. | ||
| - If the sequence contains a single element (e.g., ``['//']``), | ||
| the same pattern is repeated for all datasets. | ||
| - Single string values (e.g., ``'//'``) are **not supported**. | ||
|
timhoffm marked this conversation as resolved.
Outdated
|
||
|
|
||
| Raises | ||
| ------ | ||
| ValueError | ||
| If ``hatch`` is a single string or a non-iterable value. | ||
|
|
||
|
timhoffm marked this conversation as resolved.
Outdated
|
||
| **kwargs : `.Rectangle` properties | ||
|
|
||
| %(Rectangle:kwdoc)s | ||
|
|
@@ -3318,6 +3333,45 @@ def grouped_bar(self, heights, *, positions=None, group_spacing=1.5, bar_spacing | |
| # TODO: do we want to be more restrictive and check lengths? | ||
| colors = itertools.cycle(colors) | ||
|
|
||
| if hatch is None: | ||
| # No hatch specified: disable hatching entirely by cycling [None]. | ||
| hatches = itertools.cycle([None]) | ||
| # TODO: Discussion — | ||
| # Should grouped_bar() apply a default hatch pattern (e.g., '//') | ||
| # when none is provided ? | ||
|
timhoffm marked this conversation as resolved.
Outdated
|
||
|
|
||
| elif isinstance(hatch, str): | ||
| raise ValueError("'hatch' must be a sequence of strings " | ||
| "(e.g., ['//']) or None; " | ||
| "a single string like '//' is not allowed." | ||
| ) | ||
|
|
||
| else: | ||
| try: | ||
| hatch_list = list(hatch) | ||
| except TypeError: | ||
| raise ValueError("'hatch' must be a sequence of strings" | ||
| "(e.g., ['//']) or None") from None | ||
|
|
||
| if not hatch_list: | ||
| # Empty sequence → treat as no hatch. | ||
| hatches = itertools.cycle([None]) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's raise instead. Semantically,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense semantically — None is the canonical way to disable hatching.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather argument, that it's likely an error in the programmatic code if it returns an empty list. I don't see a practical case in which one would naturally end up with an empty list if one has a finite set of groups and want to generate hatches.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. However, in practice, empty sequences can occur naturally in data-driven workflows. For instance, configuration files often deserialize [] for optional arrays (e.g., JSON/YAML style templates), and plotting utilities may use defensive defaults like config.get("hatch", []) to avoid KeyError. Similarly, dynamically generating hatches from metadata — e.g. [meta.get("hatch") for meta in datasets] — can yield an empty list when all datasets omit that field.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @timhoffm Waiting for your response. What did you decide regarding this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, @timhoffm — I appreciate your detailed perspective. I’d like to clarify why I still believe that gracefully handling an empty sequence (with a warning) can be beneficial in the long run, both pragmatically and philosophically. 1. Defensive and user-friendly design Matplotlib’s strength has always been its tolerance toward benign irregularities. Users rely on it as a stable visualization backend even when inputs are indirectly produced — from templates, configs, or pipelines — not always hand-written by developers. Treating [] as equivalent to None (with a warning) preserves this spirit of robustness and continuity. It avoids brittle failures in higher-level tools that integrate Matplotlib as a rendering engine, especially when such inputs originate from sources beyond direct user control. 2. Real-world automation and serialization Even though empty lists may seem unlikely in manual usage, they appear quite often in automated systems: Interoperability layers — frameworks like seaborn, pandas, or holoviews may forward arguments programmatically, sometimes defaulting to empty sequences when certain aesthetics are unused. 3. Design philosophy — protecting the fragile parts From a design standpoint, the absence of current breakage does not mean the absence of potential breakage. 4. Minimal cost, maximal stability Issuing a warning — rather than raising — strikes a balanced compromise: So, while I agree that hatch=[] might not be common in hand-written code, its graceful handling can future-proof Matplotlib against subtle integration issues in complex or automated visualization systems — where inputs are often constructed indirectly rather than typed explicitly. In essence, this isn’t about accommodating an artificial case — it’s about maintaining the robustness and user-centered flexibility that have always defined Matplotlib’s design philosophy. I’d like to open a separate issue to discuss this more broadly, since the handling of empty sequences (like hatch=[]) may have implications beyond this specific function. This could be an opportunity to evaluate whether Matplotlib should adopt a consistent, system-wide approach toward benign empty inputs — for example, treating them as “no-op” equivalents with a warning, rather than raising outright. Such a discussion could help clarify the library’s general stance on defensive input handling versus strict validation, especially in the context of data-driven or programmatic pipelines that interface with Matplotlib. Personally, as a user, it feels more natural and intuitive to assign hatch = [], since it allows me to dynamically add or remove patterns later
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recognize you are trying to make your point. However, you are not responding to my points in #30726 (comment). In particular, please stop posting AI-generated content. A generic AI argument falls short of considering the nuances and context of API design here. I am not spending time on rebutting AI gibberish. (I'm sorry if my AI assumption is wrong, but the text feels very AI-generated and scanners say with 99% likelihood it is.) These principles found my decision:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just wanted to point out a small consistency angle that might be worth considering. If None is accepted to mean “no hatch,” then treating an empty list [] the same way actually keeps the API simple rather than expanding it — both essentially express “no pattern.” For example: In a lot of configuration-driven setups (like JSON or YAML), optional arrays often deserialize as empty lists by default instead of None. Handling [] like None would make the behavior more predictable and tolerant in those cases, without changing the meaning or adding any new semantics. This isn’t about guessing what the user wants — it’s more about keeping the two representations of “no hatch” consistent in data-driven workflows, where either form might appear naturally. For instance, in dynamic plotting code, users might deliberately pass [] when they’re building visual attributes incrementally or conditionally: In this kind of logic, hatches starts as an empty list by design — it’s not an error or a missing value, just a placeholder that may or may not be populated later depending on conditions. Passing [] deliberately keeps the code clean and avoids special-case checks like:
So, supporting [] as equivalent to None helps in situations where users or frameworks construct arguments dynamically, keeping code simpler and more consistent without altering semantics.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There can be - IMHO rare - cases where this is convenient. But that’s not enough reason for me to be more permissive, given it potentially hides unintended behavior and is not in line with how stackplot operates (our most prominent existing multi-dataset plotting function). I wield my decision authority as the project API lead here to end the discussion so that we can move forward. It is possible to reconsider later, and broaden the API if strong user becomes apparent. That would be a global API decision and affect stackplot and possibly Collections as well.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, Tim — I appreciate the clear decision and your time in walking through the reasoning. Understood on keeping the API strict for now and prioritizing consistency with stackplot. I won’t push the point further, and I’ll make the required changes to the code soon. |
||
| elif not all(isinstance(h, str) for h in hatch_list): | ||
| raise TypeError("All entries in 'hatch' must be strings") | ||
|
timhoffm marked this conversation as resolved.
Outdated
|
||
| else: | ||
| # Sequence of hatch patterns: cycle through them as needed. | ||
| # Example: hatch=['//', 'xx', '..'] → patterns repeat across datasets. | ||
| hatches = itertools.cycle(hatch) | ||
|
|
||
| # TODO: Discussion — | ||
| # We may later introduce optional strict validation: | ||
| # if len(hatch) != num_datasets: | ||
| # raise ValueError( | ||
| # f"Expected {num_datasets} hatches, got {len(hatch)}" | ||
| # ) | ||
| # This would enforce a strict 1:1 correspondence between | ||
| # datasets and provided hatches, preventing silent cycling. | ||
|
timhoffm marked this conversation as resolved.
Outdated
|
||
|
|
||
|
Comment on lines
+3332
to
+3361
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you condense the whitespace please?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, why not just follow the pattern in hist and allow a string (since we're allowing 1d list) and therefore simplify the error checking - basically bar handles the hatch validation instead of grouped bar.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When hatch is given as a single string (e.g. "//"), we raise a ValueError. This prevents Matplotlib from incorrectly iterating over individual characters ('/', '/') instead of treating the hatch as a single pattern.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/matplotlib/matplotlib/blob/dedfe9be48ad82cade86766ef89410844ff09b31/lib/matplotlib/axes/_axes.py#L7560C8-L7560C76 --->>>>>> the implementation cycles hatch patterns per patch, which breaks grouped bar semantics because each dataset generates multiple patches. This causes the hatch sequence to repeat incorrectly within the same dataset.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, I think would also then allow for easier expansion into nesting (hatch per patch per dataset) if we wanted to go that direction.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MNT]: Discussion : Normalize hatch patterns per dataset instead of per patch in grouped_bar #30789 I’ve created a new issue to address the hatch-normalization behavior in grouped_bar (i.e., switching from per-patch hatch cycling to per-dataset hatch assignment). This allows us to track that behavioral change separately from PR #30726, which is already large and focused on adding hatch support. Splitting this out avoids scope creep, keeps the current PR reviewable, and ensures the semantic change to hatch handling receives dedicated discussion and review.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ilakkmanoharan I closed that issue b/c that behavioral discussion is necessary in this PR to move this PR forward - it's not scope creep b/c it's inherent to how you're handling the hatch argument and which errors to throw. If the current behavior of grouped hatch is one color per dataset, than the behavior of hatch should also be one color per dataset. This is also consistent with stackplot. Also, that's the behavior you've documented:
Until you get better intuition for what would make a good issue, I strongly recommend you ask maintainers if a discussion warrants a stand alone issue before opening a new one. |
||
| bar_width = (group_distance / | ||
| (num_datasets + (num_datasets - 1) * bar_spacing + group_spacing)) | ||
| bar_spacing_abs = bar_spacing * bar_width | ||
|
|
@@ -3331,15 +3385,24 @@ def grouped_bar(self, heights, *, positions=None, group_spacing=1.5, bar_spacing | |
| # place the bars, but only use numerical positions, categorical tick labels | ||
| # are handled separately below | ||
| bar_containers = [] | ||
| for i, (hs, label, color) in enumerate(zip(heights, labels, colors)): | ||
|
|
||
| # Both colors and hatches are cycled indefinitely using itertools.cycle. | ||
| # heights and labels, however, are finite (length = num_datasets). | ||
| # Because zip() stops at the shortest iterable, this loop executes exactly | ||
| # num_datasets times even though colors and hatches are infinite. | ||
| # This ensures one (color, hatch) pair per dataset | ||
| # without explicit length checks. | ||
|
ilakkmanoharan marked this conversation as resolved.
Outdated
|
||
| for i, (hs, label, color, hatch_pattern) in enumerate( | ||
| zip(heights, labels, colors, hatches) | ||
| ): | ||
| lefts = (group_centers - 0.5 * group_distance + margin_abs | ||
| + i * (bar_width + bar_spacing_abs)) | ||
| if orientation == "vertical": | ||
| bc = self.bar(lefts, hs, width=bar_width, align="edge", | ||
| label=label, color=color, **kwargs) | ||
| label=label, color=color, hatch=hatch_pattern, **kwargs) | ||
| else: | ||
| bc = self.barh(lefts, hs, height=bar_width, align="edge", | ||
| label=label, color=color, **kwargs) | ||
| label=label, color=color, hatch=hatch_pattern,**kwargs) | ||
| bar_containers.append(bc) | ||
|
|
||
| if tick_labels is not None: | ||
|
|
||

Uh oh!
There was an error while loading. Please reload this page.