Add hatchcolor parameter for Collections by Impaler343 · Pull Request #29044 · matplotlib/matplotlib · GitHub

PR summary

Follow on PR to #28104 helping to fix issues #26074 and #7059
Added hatchcolor parameter for Collections, to be able to separately control edgecolors and hatchcolors.

The fallback logic for hatchcolor is identical to the previous PR, where it follows this precedence order:

hatchcolor parameter -> hatch.color rcParam -> inherit from edgecolors if hatch.color is edge -> default to patch.edgecolor rcParam if edgecolors is not specified.

If hatchcolor parameter is specified, it will take precedence over hatch.color rcParam and edgecolors.
If hatchcolor is not specified, it will try to use hatch.color rcParam. If this rcParam is a valid color, it will be used for the hatches.
If hatch.color rcParam is edge, the hatches will inherit the color from edgecolors if it is specified.
If edgecolors is not specified, the hatches will default to patch.edgecolor rcParam.

Need to rewrite the failing test(s)
Need to write tests
What's New note
Example demos

PR checklist

"closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
Plotting related features are demonstrated in an example
New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

Thanks for keeping at this, does #28104 need to be merged first?

Yes, preferably, so that the changes for Patches can be accurately replicated for Collections

There is currently one test which is failing and might need a rewrite: lib/matplotlib/tests/test_axes.py::test_contour_hatching

Previous Behavior

In collections, hatchcolor is set along with edgecolor.

In the cases where edgecolors is explicitly specified by the user, hatchcolor is set to the first color in edgecolors and it also uses the alpha value specified by the user. However, if edgecolors is not specified, hatchcolor uses mpl.rcParams['hatch.color'] for the color, but it doesn't use the alpha value specified by the user.

New Behavior

The new implementation in this PR has hatchcolor separated from edgecolor and it uses the alpha value specified by the user, regardless of whether edgecolors is specified or not.

Reason for New Behavior

alpha works on hatchcolor when edgecolors is specified. Therefore, I believe that it should also work when edgecolors is not specified.

In the above test, alpha is set to 0.5. Ideally, the hatch should have an alpha value of 0.5, but it has the default alpha value of 1.0.

pinging @story645 for review

The PDF test for test_axes.py::test_contour_hatching isn't failing, so the baseline PDF is left untouched. I suspect that this is because alpha on hatches on PDF backend isn't completely implemented and it looks like #17049 aims to fix this.

It's somewhat unfortunate that we have to put the parameter at the end of each function signature and make it optional on all lower-level functions. But I suppose that is the price we have to pay to keep API backward compatibility. I would have to check how public all this is and whether we could eventually migrate to a cleaner API.

AFAICS you haven't modified the test. This would mean, whe have a change of default behavior. Can that be avoided?

I think that we can't avoid this change, because the default behavior is somewhat inconsistent, as explained in this comment.

(2) I suggest to modifying the test instead (likely by setting an explicit hatch color) so that the tests images do not need update. Generally, we try to use as few images as possible and changes them as a rarely as possible, because the images bloat the git repo.

I think it might be better to update the images, because it looks like the test was previously testing the incorrect behavior. I think that it is better to have the test images reflect the correct behavior, as this test covers the behavior of hatchcolors with alpha values more comprehensively than if we change the test to explicitly set the hatch color to black with alpha = 1.0.

When explicitly setting hatchcolor, I wasn't able to force alpha as 1.0 for hatchcolor without using _hatchcolors attribute.

diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
index 38857e846c..5764d3fff6 100644
--- a/lib/matplotlib/tests/test_axes.py
+++ b/lib/matplotlib/tests/test_axes.py
@@ -2651,6 +2651,9 @@ def test_contour_hatching():
                 cmap=mpl.colormaps['gray'],
                 extend='both', alpha=0.5)
    
+    for c in ax.collections:
+        c._hatchcolors = mpl.colors.to_rgba_array('black', 1.0)
+
 
 @image_comparison(
     ['contour_colorbar'], style='mpl20',

Here is why I think that the test was previously testing the incorrect behavior:
For a stackplot, with alpha = 0.3

	Previous Behavior	New Behavior (PR)
edgecolor = 'black'
edgecolor = None

I think that if we want to move this PR forward, the realistic short-term approach is to do API support sniffing via the empty call (and not even warn on third-parties for now, at least until we can agree on the long-term plan and likely also add support for multiple hatches).

… suggested changes.

Thanks @Impaler343 for adding the API sniffing. Unfortunately this is not enough, because you choose to just not pass hatchcolors at all if they are unsupported. While the idea of just drawing single-color hatches on 3rd-party backends may be reasonable (I'm fine with that), it means that these backends will try to consult rcParams["hatch.color"] to select a color; and since #28104 they can see the new unnormalized value "edge", for which there was no support before, and they will crash there. At least this value should be normalized (e.g. to the first edgecolor, I guess); ideally if hatchcolors is not supported, the draw_path_collection call should be expanded as in ContourSet.draw to draw one path at a time.

This works now on mplcairo. Thanks for going through all the work!
I haven't carefully reviewed the implementation yet but the general approach looks fine to me. Perhaps add a note that the current new API is provisional, as I expect that it will change in the future at least to support multiple hatches as well? (I would rather only change things on mplcairo's side once multiple hatches are supported too.)

@anntzer Are there any other changes expected? Or is this ready to be merged?

Just one last suggestion to improve on the future extensibility (approval is conditional on at least discussing it).
I still don't really like the API but it's also unrealistic to hope for a full rewrite here, so let's go with it.

…gument

I still don't really like the API but it's also unrealistic to hope for a full rewrite here, so let's go with it.

I agree that this API will need to be changed in the future.

@timhoffm any suggestions?

Thanks for this PR - it looks great overall, and the examples were super nice to understand the new feature.

There's a few minor points I think can be improved, and some questions I had - I've left my comments inline in the code.

codecov is saying that these lines aren't covered (and several other lines in this file). Could you add some tests for these lines? They don't have to be figure tests, they can just test that the code works without erroring.

These lines are run only in the case of a third-party backend. However, I am not sure how to test this within the current testing framework of matplotlib, without a third-party backend.

You can make a mock third party backend to call the function - there are some examples of other mocks in the tests

Suggested change

      
                        # Find whether renderer.draw_path_collection() takes hatchcolor parameter
          
                        # Find whether renderer.draw_path_collection() takes hatchcolor parameter.
          
                        # Since third-party implementations of draw_path_collection() may not be
          
                        # introspectable, e.g. with inspect.signature, the only way is to try and
          
                        # call this with the hatchcolors parameter.

It's worth explicitly documenting this in the comment, see suggestion above.

Is this a red herring? From what I understand from #29044 (comment), making hatchcolors optional (or even kw-only) does not buy us anything in terms of better API compatibility: This is reimplemented by backends. Some may have implemented a hatchcolors parameter, others may not. It is only called from our matplotlib code, so we anyway have to deal with it being available or not (and if it's available, we always want to pass hatchcolors). There shouldn't be any third-party/user code needing to call this function.

If that's correct, an additional positional parameter would have the same effect. And then I'd prefer that because it keeps the whole function consistent.

ping @anntzer Am I correct here?

Edit: There seem to be very few "real" usages of draw_path_collection (I've tried to filter outforks or copies of our files):
https://github.com/search?q=%2F%5C.draw_path_collection%5C%28%2F+language%3APython+NOT+is%3Afork+NOT+repo%3Amatplotlib%2Fmatplotlib+NOT+path%3A**%2Fbackend_bases.py+NOT+path%3A**%2Fcollections.py++NOT+path%3A**%2Fpatheffects.py+NOT+path%3A**%2Fbackend_agg.py+NOT+path%3A**%2Fbackend_svg.py+NOT+path%3A**%2Fbackend_pdf.py++NOT+path%3A**%2Fbackend_ps.py+NOT+path%3A**%2Fbackend_macosx.py++NOT+path%3A**%2Fleft.py+NOT+path%3A**%2Fright.py++NOT+path%3A**%2Fmerge.py++NOT+path%3A**%2Fbase.py+NOT+path%3A**%2Fsite-packages%2F**&type=code

I think making this optional is meaningful for a start because it will support some very rare use cases. But in terms of API consistency, I'd like to move this to a regular parameter in the future, i.e.

not make this kw-only right now
some time later deprecate calling this function without hatchcolors

@anntzer @dstansby @r3kste Does that make sense?

re: making hatchcolors optional and API compatibility:
My point of view is that the renderer API (draw_foo) is public and it is legitimate for 3rd-parties to call it directly (e.g. they can create their own artist subclasses that implement their own draw() and call draw_foo() from there. However, I guess it could also be reasonable(?), and possibly helpful for extensibility, to declare that these methods are not for 3rd-party direct use, and that third-party artists must go through a builtin artist's draw to invoke the renderer's drawing functions. But that's definitely not a policy we have stated so far.

Thanks. I agree optional is set to begin with for compatibility.

Questions are:

Do we want/gain anything with keyword-only? - I tend towards no.
Do we want to evolve the API to non-optional through regular deprecation machinery? - I tend towards yes.

Do we want/gain anything with keyword-only? - I tend towards no.

It makes it easier to later include parameters before it, e.g. hatches is something that's clearly missing and would be expected to come first.

Do we want to evolve the API to non-optional through regular deprecation machinery? - I tend towards yes.

Let's not for now. As argued above I think the API introduced in this PR is actually not so great (but still the best that can be done without an in-depth reworking of draw_path_collection), so e.g. I'd rather not implement support for it at all in mplcairo (and instead rely on Matplotlib's fallback implementation) until a better API comes. Likewise I don't think we should encourage any third parties to implement it.

…tchcolors arg

For better backward-compatibility/performance, would it be reasonable to split the "backend does not support hatchcolors" into

if no_hatches_needed:
    renderer_draw_path_collection(...) # no hatch colors passed
else:
    # unroll the collection to draw paths in a loop

I'm unclear how performance-critical this is and whether we can unversally force the collection unrolling on backends that do not support hatchcolors.

ping @anntzer

Good catch. I agree that only going through the unrolled version when hatchcolors are actually passed would likely be much better performance-wise.

Thanks @r3kste for going through all the nitty gritty details with us!

github-actions Bot added topic: rcparams backend: agg backend: ps backend: pdf backend: svg topic: legend topic: collections and mappables topic: styles topic: canvas and figure manager labels Oct 30, 2024

story645 added the status: waiting for other PR label Oct 31, 2024

github-actions Bot added the status: needs rebase label Oct 31, 2024

Impaler343 mentioned this pull request Dec 19, 2024

Separates edgecolor from hatchcolor #28104

Merged

6 tasks done

Impaler343 marked this pull request as ready for review January 3, 2025 11:58

r3kste force-pushed the collections-hatchcolor branch from ab4081d to 256d735 Compare January 3, 2025 12:17

github-actions Bot removed topic: rcparams topic: styles status: needs rebase labels Jan 3, 2025

github-actions Bot added the status: needs rebase label Jan 4, 2025

timhoffm removed the status: waiting for other PR label Jan 5, 2025

r3kste force-pushed the collections-hatchcolor branch from c8d7b85 to b081bb9 Compare January 8, 2025 15:52

github-actions Bot added Documentation: examples files in galleries/examples and removed status: needs rebase labels Jan 8, 2025

timhoffm reviewed Jan 25, 2025

View reviewed changes

Comment thread lib/matplotlib/tests/baseline_images/test_axes/contour_hatching.png

timhoffm reviewed Jan 25, 2025

View reviewed changes

Comment thread doc/users/next_whats_new/separated_hatchcolor.rst

pass hatchcolor arg only if it is supported by the renderer. and made…

7e46707

… suggested changes.

Fix hatchcolors argument support in third-party backends

c5e9583

Add note about provisional API for draw_path_collection()

d2b6268

anntzer reviewed Mar 16, 2025

View reviewed changes

Comment thread lib/matplotlib/backend_bases.py Outdated

anntzer approved these changes Mar 16, 2025

View reviewed changes

Refactor draw_path_collection to make hatchcolors a keyword-only ar…

fee895b

…gument

dstansby reviewed Mar 21, 2025

View reviewed changes

Made suggested changes

b9ac0fb

timhoffm reviewed Mar 24, 2025

View reviewed changes

Impaler343 force-pushed the collections-hatchcolor branch from 9800478 to 0c557e4 Compare March 28, 2025 15:54

added test and improve comments for third party backend support of ha…

2e4784b

…tchcolors arg

r3kste force-pushed the collections-hatchcolor branch from 0c557e4 to 2e4784b Compare March 28, 2025 15:55

timhoffm reviewed Mar 28, 2025

View reviewed changes

skip unnecessary path iteration when hatchcolors is not passed/needed

3b43bc6

timhoffm approved these changes Mar 31, 2025

View reviewed changes

timhoffm added this to the v3.11.0 milestone Mar 31, 2025

timhoffm merged commit 65d2818 into matplotlib:main Mar 31, 2025

anntzer mentioned this pull request Apr 7, 2025

[Bug]: Missing backcompat for backends not supporting hatchcolors in draw_quad_mesh #29883

Closed

story645 mentioned this pull request May 18, 2025

Fixed hatching in PatchCollection class #27937

Open

4 tasks done

r3kste mentioned this pull request Sep 11, 2025

[MNT]: Revamp draw_path_collection() API #30547

Open

timhoffm mentioned this pull request Sep 14, 2025

[ENH]: Backend versioning #30559

Closed

                   def draw_quad_mesh(self, gc, master_transform, meshWidth, meshHeight,
                                      coordinates, offsets, offsetTrans, facecolors,
-                                     antialiased, edgecolors):
+                                     antialiased, edgecolors, hatchcolors=None):

+                                  path_ids = renderer._iter_collection_raw_paths(
+                                      transform.frozen(), ipaths, self.get_transforms())
+                                  for xo, yo, path_id, gc0, rgbFace in renderer._iter_collection(
+                                      gc, list(path_ids), *args, hatchcolors=self.get_hatchcolor(),
+                                  ):
+                                      path, transform = path_id
+                                      if xo != 0 or yo != 0:
+                                          transform = transform.frozen()
+                                          transform.translate(xo, yo)
+                                      renderer.draw_path(gc0, path, transform, rgbFace)

+                                  "screen", hatchcolors=self.get_hatchcolor()
+                              )
+                          except TypeError:
+                              hatchcolors_arg_supported = False

+                          # The current new API of draw_path_collection() is provisional
+                          # and will be changed in a future PR.
+                          # Find whether renderer.draw_path_collection() takes hatchcolor parameter

Add hatchcolor parameter for Collections by Impaler343 · Pull Request #29044 · matplotlib/matplotlib · GitHub

Repository navigation

Add hatchcolor parameter for Collections - #29044

Add hatchcolor parameter for Collections#29044timhoffm merged 16 commits intomatplotlib:mainmatplotlib/matplotlib:mainfrom Impaler343:collections-hatchcolorImpaler343/matplotlib:collections-hatchcolorCopy head branch name to clipboard

Conversation

Impaler343 commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

PR checklist

Uh oh!

story645 commented Oct 30, 2024

Uh oh!

Impaler343 commented Oct 31, 2024

Uh oh!

r3kste commented Oct 31, 2024

Previous Behavior

New Behavior

Reason for New Behavior

Uh oh!

Impaler343 commented Jan 3, 2025

Uh oh!

r3kste commented Jan 3, 2025

Uh oh!

timhoffm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

r3kste commented Jan 26, 2025

Uh oh!

anntzer commented Feb 14, 2025

Uh oh!

anntzer commented Feb 17, 2025

Uh oh!

anntzer commented Feb 22, 2025

Uh oh!

r3kste commented Mar 15, 2025

Uh oh!

Uh oh!

anntzer left a comment

Choose a reason for hiding this comment

Uh oh!

r3kste commented Mar 21, 2025

Uh oh!

dstansby left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dstansby Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

r3kste Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

story645 Mar 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

timhoffm Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

timhoffm Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

timhoffm Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

anntzer commented Mar 24, 2025

Add hatchcolor parameter for Collections#29044
timhoffm merged 16 commits into
matplotlib:mainfrom
Impaler343:collections-hatchcolor

Impaler343 commented Oct 30, 2024 •
edited
Loading

timhoffm left a comment •
edited
Loading

anntzer Mar 29, 2025 •
edited
Loading