Skip to content
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

[Bug]: Hatch linewidths cannot be modified in an rcParam context #21108

Open
mwaskom opened this issue Sep 17, 2021 · 24 comments · May be fixed by #28048
Open

[Bug]: Hatch linewidths cannot be modified in an rcParam context #21108

mwaskom opened this issue Sep 17, 2021 · 24 comments · May be fixed by #28048
Labels
API: consistency Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues Good first issue Open a pull request against these issues if there are no active ones! topic: hatch topic: rcparams

Comments

@mwaskom
Copy link

mwaskom commented Sep 17, 2021

Bug summary

The only way (that I am aware of) to control the linewidth of hatches is through an rc parameter. But temporarily modifying the parameter with plt.rc_context has not effect.

Code for reproduction

import matplotlib.pyplot as plt

plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="r")

with plt.rc_context({"hatch.linewidth": 5}):
    plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="g")

plt.rc("hatch", linewidth=5)
plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="b")

Actual outcome

image

image

image

Expected outcome

That second image (the green bars) should have thick hatches.

FWIW I think hatches ought to have an actual API, but given that they don't, this limitation makes them really hard to work with.

Operating system

macos

Matplotlib Version

3.4.3

Matplotlib Backend

inline

@story645
Copy link
Member

story645 commented Sep 17, 2021

FWIW I think hatches ought to have an actual API

totally agree that there should be a HatchStyle object that holds the

  • pattern
  • width
  • color
    any other attributes of the hatch...

@mwaskom
Copy link
Author

mwaskom commented Sep 17, 2021

I guess this is not the first time I've noticed this :)

@jklymak
Copy link
Member

jklymak commented Sep 17, 2021

... someone who uses hatches has to implement the new API, but I think it would be a welcome addition....

@mwaskom
Copy link
Author

mwaskom commented Sep 17, 2021

I agree but this apparently does work for hatch.color:

import matplotlib.pyplot as plt

plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="r")

with plt.rc_context({"hatch.color": "w"}):
    plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="g")

plt.rc("hatch", color="w")
plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="b")

So I am hoping that there is a small bug with hatch.linewidth where it is getting read from the rcparams at the wrong time. A proper hatching API has been on the agenda for years now with no one stepping up to take it on so I don't think that should stand in the way of sanding down rough edges where possible.

@timhoffm
Copy link
Member

Indeed, hatch.linewidth is only used within backend code, which makes it clear that it's evaluated at draw time and thus the context manager cannot have any effect. In contrast, Patch and Collection store a self._hatch_color during creation.

Marking as good first issue, because the implementation of hatch.linewidth handling must only be adapted to be analogous to hatch.color.

@timhoffm timhoffm added Good first issue Open a pull request against these issues if there are no active ones! API: consistency topic: rcparams Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues labels Sep 17, 2021
@jklymak
Copy link
Member

jklymak commented Sep 18, 2021

Seems reasonable.

However I wonder if we have somewhere we could codify draw parameter resolution. For all the people who want to use a context manager and have properties set at creation, we have the equally reasonable request that rcParams are checked at drawtime so folks can style after they have created a bunch of artists. I think most of the library is consistent with "defaults resolved at creation" but I'm not sure we document this.

@anntzer
Copy link
Contributor

anntzer commented Sep 18, 2021

See also #12967, #11710: I think rcparams should be resolved at artist creation time, mostly because proper delayed resolution would be rather hard to implement given the current architecture of things.

@anntzer
Copy link
Contributor

anntzer commented Sep 18, 2021

Also, I think the patch to resolve hatch linewidth early should basically be

diff --git a/lib/matplotlib/backend_bases.py b/lib/matplotlib/backend_bases.py
index cf352af563..fa086acd64 100644
--- a/lib/matplotlib/backend_bases.py
+++ b/lib/matplotlib/backend_bases.py
@@ -1013,6 +1013,10 @@ class GraphicsContextBase:
         """Get the hatch linewidth."""
         return self._hatch_linewidth
 
+    def set_hatch_linewidth(self, hatch_linewidth):
+        """Set the hatch linewidth."""
+        self._hatch_linewidth = hatch_linewidth
+
     def get_sketch_params(self):
         """
         Return the sketch parameters for the artist.
diff --git a/lib/matplotlib/backends/backend_pdf.py b/lib/matplotlib/backends/backend_pdf.py
index bd0c370f1c..1d83396cb8 100644
--- a/lib/matplotlib/backends/backend_pdf.py
+++ b/lib/matplotlib/backends/backend_pdf.py
@@ -698,7 +698,7 @@ class PdfFile:
         self._soft_mask_states = {}
         self._soft_mask_seq = (Name(f'SM{i}') for i in itertools.count(1))
         self._soft_mask_groups = []
-        self.hatchPatterns = {}
+        self._hatch_patterns = {}
         self._hatch_pattern_seq = (Name(f'H{i}') for i in itertools.count(1))
         self.gouraudTriangles = []
 
@@ -1508,25 +1508,28 @@ end"""
     def hatchPattern(self, hatch_style):
         # The colors may come in as numpy arrays, which aren't hashable
         if hatch_style is not None:
-            edge, face, hatch = hatch_style
+            edge, face, hatch, lw = hatch_style
             if edge is not None:
                 edge = tuple(edge)
             if face is not None:
                 face = tuple(face)
-            hatch_style = (edge, face, hatch)
+            hatch_style = (edge, face, hatch, lw)
 
-        pattern = self.hatchPatterns.get(hatch_style, None)
+        pattern = self._hatch_patterns.get(hatch_style, None)
         if pattern is not None:
             return pattern
 
         name = next(self._hatch_pattern_seq)
-        self.hatchPatterns[hatch_style] = name
+        self._hatch_patterns[hatch_style] = name
         return name
 
+    hatchPatterns = _api.deprecated("3.6")(property(lambda self:
+        {k: (e, f, h) for k, (e, f, h, l) in self._hatch_patterns}))
+
     def writeHatches(self):
         hatchDict = dict()
         sidelen = 72.0
-        for hatch_style, name in self.hatchPatterns.items():
+        for hatch_style, name in self._hatch_patterns.items():
             ob = self.reserveObject('hatch pattern')
             hatchDict[name] = ob
             res = {'Procsets':
@@ -1541,7 +1544,7 @@ end"""
                  # Change origin to match Agg at top-left.
                  'Matrix': [1, 0, 0, 1, 0, self.height * 72]})
 
-            stroke_rgb, fill_rgb, hatch = hatch_style
+            stroke_rgb, fill_rgb, hatch, lw = hatch_style
             self.output(stroke_rgb[0], stroke_rgb[1], stroke_rgb[2],
                         Op.setrgb_stroke)
             if fill_rgb is not None:
@@ -1549,8 +1552,7 @@ end"""
                             Op.setrgb_nonstroke,
                             0, 0, sidelen, sidelen, Op.rectangle,
                             Op.fill)
-
-            self.output(mpl.rcParams['hatch.linewidth'], Op.setlinewidth)
+            self.output(lw, Op.setlinewidth)
 
             self.output(*self.pathOperations(
                 Path.hatch(hatch),
@@ -2502,14 +2504,15 @@ class GraphicsContextPdf(GraphicsContextBase):
         name = self.file.alphaState(effective_alphas)
         return [name, Op.setgstate]
 
-    def hatch_cmd(self, hatch, hatch_color):
+    def hatch_cmd(self, hatch, hatch_color, hatch_linewidth):
         if not hatch:
             if self._fillcolor is not None:
                 return self.fillcolor_cmd(self._fillcolor)
             else:
                 return [Name('DeviceRGB'), Op.setcolorspace_nonstroke]
         else:
-            hatch_style = (hatch_color, self._fillcolor, hatch)
+            hatch_style = (
+                hatch_color, self._fillcolor, hatch, hatch_linewidth)
             name = self.file.hatchPattern(hatch_style)
             return [Name('Pattern'), Op.setcolorspace_nonstroke,
                     name, Op.setcolor_nonstroke]
@@ -2574,7 +2577,7 @@ class GraphicsContextPdf(GraphicsContextBase):
         (('_dashes',), dash_cmd),
         (('_rgb',), rgb_cmd),
         # must come after fillcolor and rgb
-        (('_hatch', '_hatch_color'), hatch_cmd),
+        (('_hatch', '_hatch_color', '_hatch_linewidth'), hatch_cmd),
         )
 
     def delta(self, other):
@@ -2603,11 +2606,11 @@ class GraphicsContextPdf(GraphicsContextBase):
                     break
 
             # Need to update hatching if we also updated fillcolor
-            if params == ('_hatch', '_hatch_color') and fill_performed:
+            if cmd.__name__ == 'hatch_cmd' and fill_performed:
                 different = True
 
             if different:
-                if params == ('_fillcolor',):
+                if cmd.__name__ == 'fillcolor_cmd':
                     fill_performed = True
                 theirs = [getattr(other, p) for p in params]
                 cmds.extend(cmd(self, *theirs))
diff --git a/lib/matplotlib/backends/backend_ps.py b/lib/matplotlib/backends/backend_ps.py
index c44f89c638..eb7adee222 100644
--- a/lib/matplotlib/backends/backend_ps.py
+++ b/lib/matplotlib/backends/backend_ps.py
@@ -373,7 +373,6 @@ class RendererPS(_backend_pdf_ps.RendererPDFPSBase):
         if hatch in self._hatches:
             return self._hatches[hatch]
         name = 'H%d' % len(self._hatches)
-        linewidth = mpl.rcParams['hatch.linewidth']
         pageheight = self.height * 72
         self._pswriter.write(f"""\
   << /PatternType 1
@@ -385,7 +384,6 @@ class RendererPS(_backend_pdf_ps.RendererPDFPSBase):
 
      /PaintProc {{
         pop
-        {linewidth:f} setlinewidth
 {self._convert_path(
     Path.hatch(hatch), Affine2D().scale(sidelen), simplify=False)}
         gsave
@@ -802,6 +800,7 @@ grestore
         if hatch:
             hatch_name = self.create_hatch(hatch)
             write("gsave\n")
+            write("%f setlinewidth" % gc.get_hatch_linewidth())
             write("%f %f %f " % gc.get_hatch_color()[:3])
             write("%s setpattern fill grestore\n" % hatch_name)
 
diff --git a/lib/matplotlib/backends/backend_svg.py b/lib/matplotlib/backends/backend_svg.py
index e4de85905c..87b8497634 100644
--- a/lib/matplotlib/backends/backend_svg.py
+++ b/lib/matplotlib/backends/backend_svg.py
@@ -480,11 +480,13 @@ class RendererSVG(RendererBase):
         edge = gc.get_hatch_color()
         if edge is not None:
             edge = tuple(edge)
-        dictkey = (gc.get_hatch(), rgbFace, edge)
+        lw = gc.get_hatch_linewidth()
+        dictkey = (gc.get_hatch(), rgbFace, edge, lw)
         oid = self._hatchd.get(dictkey)
         if oid is None:
             oid = self._make_id('h', dictkey)
-            self._hatchd[dictkey] = ((gc.get_hatch_path(), rgbFace, edge), oid)
+            self._hatchd[dictkey] = (
+                (gc.get_hatch_path(), rgbFace, edge, lw), oid)
         else:
             _, oid = oid
         return oid
@@ -495,7 +497,7 @@ class RendererSVG(RendererBase):
         HATCH_SIZE = 72
         writer = self.writer
         writer.start('defs')
-        for (path, face, stroke), oid in self._hatchd.values():
+        for (path, face, stroke, lw), oid in self._hatchd.values():
             writer.start(
                 'pattern',
                 id=oid,
@@ -519,7 +521,7 @@ class RendererSVG(RendererBase):
             hatch_style = {
                     'fill': rgb2hex(stroke),
                     'stroke': rgb2hex(stroke),
-                    'stroke-width': str(mpl.rcParams['hatch.linewidth']),
+                    'stroke-width': str(lw),
                     'stroke-linecap': 'butt',
                     'stroke-linejoin': 'miter'
                     }
diff --git a/lib/matplotlib/collections.py b/lib/matplotlib/collections.py
index 619c62b5ca..42894a27d1 100644
--- a/lib/matplotlib/collections.py
+++ b/lib/matplotlib/collections.py
@@ -172,6 +172,7 @@ class Collection(artist.Artist, cm.ScalarMappable):
         self._edge_is_mapped = None
         self._mapped_colors = None  # calculated in update_scalarmappable
         self._hatch_color = mcolors.to_rgba(mpl.rcParams['hatch.color'])
+        self._hatch_linewidth = mpl.rcParams['hatch.linewidth']
         self.set_facecolor(facecolors)
         self.set_edgecolor(edgecolors)
         self.set_linewidth(linewidths)
@@ -362,6 +363,7 @@ class Collection(artist.Artist, cm.ScalarMappable):
         if self._hatch:
             gc.set_hatch(self._hatch)
             gc.set_hatch_color(self._hatch_color)
+            mhatch._set_hatch_linewidth(gc, self._hatch_linewidth)
 
         if self.get_sketch_params() is not None:
             gc.set_sketch_params(*self.get_sketch_params())
@@ -539,6 +541,14 @@ class Collection(artist.Artist, cm.ScalarMappable):
         """Return the current hatching pattern."""
         return self._hatch
 
+    def set_hatch_linewidth(self, hatch_linewidth):
+        """Set the hatch linewidth."""
+        self._hatch_linewidth = hatch_linewidth
+
+    def get_hatch_linewidth(self):
+        """Return the hatch linewidth."""
+        return self._hatch_linewidth
+
     def set_offsets(self, offsets):
         """
         Set the offsets for the collection.
diff --git a/lib/matplotlib/hatch.py b/lib/matplotlib/hatch.py
index c490f404c9..2a4deab2d6 100644
--- a/lib/matplotlib/hatch.py
+++ b/lib/matplotlib/hatch.py
@@ -222,3 +222,14 @@ def get_path(hatchpattern, density=6):
             cursor += pattern.num_vertices
 
     return Path(vertices, codes)
+
+
+def _set_hatch_linewidth(gc, hatch_linewidth):
+    if hasattr(gc, "set_hatch_linewidth"):
+        gc.set_hatch_linewidth(hatch_linewidth)
+    else:
+        _api.warn_deprecated(
+            "3.6", message="The current backend does not define "
+            "GraphicsContextRenderer.set_hatch_linewidth; support for such "
+            "backends is deprecated since %(since)s and will be removed "
+            "%(removal)s.")
diff --git a/lib/matplotlib/patches.py b/lib/matplotlib/patches.py
index 253e8abe2e..29737fc207 100644
--- a/lib/matplotlib/patches.py
+++ b/lib/matplotlib/patches.py
@@ -87,6 +87,7 @@ class Patch(artist.Artist):
             antialiased = mpl.rcParams['patch.antialiased']
 
         self._hatch_color = colors.to_rgba(mpl.rcParams['hatch.color'])
+        self._hatch_linewidth = mpl.rcParams['hatch.linewidth']
         self._fill = True  # needed for set_facecolor call
         if color is not None:
             if edgecolor is not None or facecolor is not None:
@@ -545,6 +546,14 @@ class Patch(artist.Artist):
         """Return the hatching pattern."""
         return self._hatch
 
+    def set_hatch_linewidth(self, hatch_linewidth):
+        """Set the hatch linewidth."""
+        self._hatch_linewidth = hatch_linewidth
+
+    def get_hatch_linewidth(self):
+        """Return the hatch linewidth."""
+        return self._hatch_linewidth
+
     @contextlib.contextmanager
     def _bind_draw_path_function(self, renderer):
         """
@@ -579,6 +588,7 @@ class Patch(artist.Artist):
         if self._hatch:
             gc.set_hatch(self._hatch)
             gc.set_hatch_color(self._hatch_color)
+            mhatch._set_hatch_linewidth(gc, self._hatch_linewidth)
 
         if self.get_sketch_params() is not None:
             gc.set_sketch_params(*self.get_sketch_params())

(the main complication is the the vector backends hatch emitters needed to be updated as well.)

Adding tests (and more annoyingly, backcompat knobs) is left as an exercise to the reader.

@Fefedu75
Copy link

Hello ! We are students from an Ingenier school and we are specializing in open source software. We hoped help you, is this problem still unsolved ?

@tacaswell
Copy link
Member

@Fefedu75 Yes, this issue is still unresolved. The work that needs to be done is to convert the patch in the issue above into a PR, add tests, and add documentation of the new feature.

@aitikgupta aitikgupta added the hacktoberfest-accepted valid for hacktoberfest but not ready for merging label Oct 2, 2021
@singhaditya28
Copy link

singhaditya28 commented Oct 7, 2021

@tacaswell Which all docs need to be updated can you tell me?

@tacaswell
Copy link
Member

Please see the development guide: https://matplotlib.org/stable/devel/coding_guide.html#pr-documentation

@tacaswell tacaswell added this to the v3.6.0 milestone Oct 7, 2021
@jklymak jklymak removed the hacktoberfest-accepted valid for hacktoberfest but not ready for merging label Oct 12, 2021
@QuLogic QuLogic modified the milestones: v3.6.0, v3.7.0 Aug 24, 2022
@QuLogic QuLogic modified the milestones: v3.7.0, future releases Jan 25, 2023
@rishavganguly007
Copy link

Hello,

I tried to reproduce the code mentioned in the ticket

import matplotlib.pyplot as plt

plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="r")

with plt.rc_context({"hatch.linewidth": 5}):
    plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="g")

plt.rc("hatch", linewidth=5)
plt.figure().subplots().bar([0, 1], [1, 2], hatch=["/", "."], fc="b")

However I am getting a different result:

image
image

Unlike the original posted images, I'm getting equal linewidths for all the 3 hatches

Here is my collab document : https://colab.research.google.com/drive/14WAgV-hVgmjIaBhqw6UQI1wBMjO5FXrq#scrollTo=846DkmimmGaL

I have tested in:
Name: matplotlib
Version: 3.5.3

So what I wanted to know is the issue fixed with the new versions?

Thanks

@rcomer
Copy link
Member

rcomer commented Feb 26, 2023

Thanks for checking this @rishavganguly007! However, I think for the first (red) subplot the hatches should be thinner. So something else seems to be going wrong in your example 🤔

@rcomer
Copy link
Member

rcomer commented Feb 26, 2023

@rishavganguly007 I think if you put the three calls in separate cells in your notebook, you might reproduce the problem in the OP. I think the call to rc on the penultimate line affects everything that is drawn for that cell.

@rishavganguly007
Copy link

I think if you put the three calls in separate cells in your notebook, you might reproduce the problem in the OP.

Indeed @rcomer, separating 3 calls in 3 different cells reproduces the OP's issue. Thanks.

However, a question arises here, so the call to rc on the penultimate line - why It affected the other two calls that were placed above it? Is it something related to Google Colab, or is this how it is?, or it's a bug?

Coming to the issue,

What I understood reading from comments, can you please confirm that is this what is expected from this issue?.

@Fefedu75 Yes, this issue is still unresolved. The work that needs to be done is to convert the patch in the issue above into a PR, add tests, and add documentation of the new feature.

Thanks

@rcomer
Copy link
Member

rcomer commented Feb 26, 2023

so the call to rc on the penultimate line - why It affected the other two calls that were placed above it? Is it something related to Google Colab, or is this how it is?, or it's a bug?

It’s not specific to Google Colab: I found the same thing by running the example as a script locally: if I put plt.show() once at the end of the script, then my three figures pop up with thick hatches an all of them. If instead I add plt.show() after each call to bar, then I get the figures one at a time, and they look like those in the OP. I think that the rcParam affects what happens at draw time, (i.e. when you call show or savefig in a script) rather than affecting attributes on the Figure objects. In a notebook, draw is called implicitly at the end of a cell.

What I understood reading from comments, can you please confirm that is this what is expected from this issue?.

@Fefedu75 Yes, this issue is still unresolved. The work that needs to be done is to convert the patch in the issue above into a PR, add tests, and add documentation of the new feature.

Yes I think so. Someone made a start at #21325, but it seems that got caught up in some gnarly git history problems. So it may well be easiest to start again.

@mwaskom
Copy link
Author

mwaskom commented Feb 26, 2023

You've discovered a different manifestation of the bug as reported in the issue. The hatch linewidths are determined by the state of the rcParams at the time the figure is drawn not at the time the artists are created (as is the case for most other visual attributes in matplotlib). That's why the context manager doesn't work (the figure is drawn after the parameters revert back to their original values) and why changing the global settings in a cell retroactively modifies figures (because the inline backend collects all open figures and draws them after executing the cell).

@kidkoder432
Copy link
Contributor

I see that the PR above (#25367) has been closed due to stalling on some test failures. Should it be reopened, so I can fix the failures, or should I start from scratch on a completely new PR?

@oscargus
Copy link
Contributor

oscargus commented Oct 6, 2023

The best thing is to start a new PR which uses the commits from the old one.

@kidkoder432
Copy link
Contributor

kidkoder432 commented Oct 6, 2023 via email

@kidkoder432
Copy link
Contributor

kidkoder432 commented Oct 7, 2023

Adding tests (and more annoyingly, backcompat knobs) is left as an exercise to the reader.

I've read through this diff and applied the changes described. How do I implement tests for this? Also, what exactly are backcompat hooks, and how would I implement them?

@kidkoder432
Copy link
Contributor

Adding tests (and more annoyingly, backcompat knobs) is left as an exercise to the reader.

I've read through this diff and applied the changes described. How do I implement tests for this? Also, what exactly are backcompat hooks, and how would I implement them?

@tacaswell @anntzer

@rcomer
Copy link
Member

rcomer commented Oct 8, 2023

backcompat knobs: if we make a change that could alter the outcome of a user’s existing code in some way, then we must not do that without warning. Guidelines for this are at https://matplotlib.org/stable/devel/contribute.html#api-changes

We have guidance on writing tests here: https://matplotlib.org/stable/devel/testing.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: consistency Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues Good first issue Open a pull request against these issues if there are no active ones! topic: hatch topic: rcparams
Projects
None yet