ENH: enable stripey lines #23208

rcomer · 2022-06-05T19:21:39Z

PR Summary

Closes #18351 and closes #19053, which had been orphaned, so this replaces. This PR implements a keyword to set a second colour for a line, and create a stripey effect.

The first two commits are cherry-picked from #19053, with some tweaks to address conflicts with the #21050 changes. I'm happy to squash everything down to one commit, but thought I'd leave them separate for now so that my changes are more transparent.

Two outstanding questions:

As per Add the ability to plot striped lines #19053 (review), is offcolor the best name for this new keyword? At Add the ability to plot striped lines #18351, alt_color, coloralt and gapcolor were suggested. coloralt would be consistent with markerfacecoloralt, though gapcolor would seem to me the most consistent with things like facecolor and edgecolor. Since the keyword will only have an effect for dashed styles, I wonder if it should be something like dash_gapcolor for consistency with dash_capstyle and dash_joinstyle.
Would the new gallery example fit better under "Lines, bars and markers" rather than "Pyplot"?

PR Checklist

Tests and Styling

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

New features are documented, with examples if plot related.
New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
[N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs should build without error).

oscargus · 2022-06-05T19:46:07Z

lib/matplotlib/lines.py

+        """
+
+        if offcolor is not None:
+            if not is_color_like(offcolor) and offcolor != 'auto':


Is there a reason to not use mcolors._check_color_like(color=color) as is done in set_color?

Looks like set_color changed since the original PR. I completely missed that and will change it.

oscargus · 2022-06-05T19:46:41Z

lib/matplotlib/lines.py

@@ -1040,7 +1078,7 @@ def set_drawstyle(self, drawstyle):
        Parameters
        ----------
        drawstyle : {'default', 'steps', 'steps-pre', 'steps-mid', \
-'steps-post'}, default: 'default'
+        'steps-post'}, default: 'default'


This should not be here as it is a continuation of the previous line.

oscargus · 2022-06-05T19:47:05Z

lib/matplotlib/lines.py

@@ -312,14 +315,12 @@ def __init__(self, xdata, ydata,

        if linewidth is None:
            linewidth = rcParams['lines.linewidth']
-


Please do not remove empty lines.

oscargus · 2022-06-05T19:50:30Z

Thanks for reviving this!

I think it would be nice if the test image included a legend and a custom dash.

Edit, you mentioned this: ~~At some stage, this will have to be squashed to get rid of the unclean error (some file was added in one commit and removed in another), but that can be done while merging.~~

Edit: Yes, it would be better to add it in some other example and ideally use something like:

fig, ax = plt.subplots()
ax.plot(..)

oscargus · 2022-06-05T20:06:33Z

Oh, and I think this warrant an entry in doc/users/next_whats_new so users can benefit from this! Ideally, a short description of the feature with a small example, see e.g. https://github.com/matplotlib/matplotlib/blob/main/doc/users/next_whats_new/custom_cap_widths.rst

rcomer · 2022-06-05T20:37:54Z

Yes, it would be better to add it in some other example

Perhaps an extra line or two in Customizing dashed line styles?

oscargus · 2022-06-06T10:53:33Z

Yes, it would be better to add it in some other example

Perhaps an extra line or two in Customizing dashed line styles?

Sounds like a great idea!

oscargus · 2022-06-06T11:00:50Z

And I agree with you that gapcolor seems like the best option here, but I would probably not risk changing it before someone else has a say as well. If noone chips in before Thursday, I will discuss it at the developer meeting.

(Feel free to join, it is open, there is also a "new contributor meeting" tomorrow: https://hackmd.io/Rvc8Dl2RSi-AsFDgj2rdLw
Although that is not in the calendar: https://calendar.google.com/calendar/u/0/embed?src=79hk8jhvlks8jn8ds4ri1e6q4g@group.calendar.google.com&ctz=America/New_York&pli=1 )

rcomer · 2022-06-06T19:53:33Z

Thanks @oscargus for your review. I think I have addressed everything except the parameter name. I recycled the old gallery example into the "what's new" entry.

greglucas · 2022-06-07T01:52:51Z

lib/matplotlib/lines.py

+                if self.is_dashed() and self._offcolor is not None:
+                    lc_rgba = mcolors.to_rgba(self._offcolor, self._alpha)
+                    gc.set_foreground(lc_rgba, isRGBA=True)
+                    gc.set_dashes(0, None)


I don't think this is enough. This will just draw a solid line in the background and doesn't look good if you set an alpha on the line. I think you'll need to unwrap the dash pattern and do the inverse dashed pattern.

Good point! It also strikes me that the dash cap style can cause some issues here. But I guess that one can probably live with that having a round dash cap style with alpha gives a less than ideal result?

The option would be, I guess, to introduce an "inverted" round cap style. Also, projecting caps will require a bit of extra work here, but should be more easily doable since one knows the additional overlap (linewidth/2).

So my initial attempt at this failed badly because I assumed that offset would add an extra "gap" to the start of the sequence, when in fact it seems to remove a section from the start instead. I couldn't find this documented anywhere: this docstring links to a "blue book" that might help, but that pdf seems to be only available on certain high security systems.

I've sanity checked this by adapting the linestyle demo, plotting with and without an initial offset of 1, so I know it works for a variety of dash styles:

It also turned out that the line underneath was problematic even without setting the alpha: the previous test image showed outlines around the lines which disappeared when I updated the main code:

I couldn't find this documented anywhere

I was under the same impression as you. That is clearly something that can be documented more clearly. Although now that I know it, I can sort of understand what offset means, and explain this line in the code:

matplotlib/lib/matplotlib/lines.py

Line 57 in 8e2987b

offset %= dsum

I opened #23220 to track this.

when in fact it seems to remove a section from the start instead. I couldn't find this documented anywhere: this docstring links to a "blue book" that might help, but that pdf seems to be only available on certain high security systems.

https://www.adobe.com/jp/print/postscript/pdfs/PLRM.pdf page 667

The offset operand can be thought of as the “phase” of the dash pattern relative to
the start of the path. It is interpreted as a distance into the dash pattern (measured
in user space units) at which to start the pattern. Before beginning to stroke a
path, the stroke operator cycles through the elements of array, adding up dis-
tances and alternating dashes and gaps as usual, but without generating any out-
put. When the accumulated distance reaches the value specified by offset, it begins
stroking from the starting point of the path, using the dash pattern from the point
that has been reached. Each subpath of a path is treated independently; the dash
pattern is restarted and the offset reapplied at the beginning of each subpath.

greglucas · 2022-06-07T01:56:43Z

lib/matplotlib/lines.py

@@ -204,6 +204,7 @@ def _slice_or_none(in_v, slc):
 @_api.define_aliases({
    "antialiased": ["aa"],
    "color": ["c"],
+    "offcolor": ["oc"],


I'm not sure this should get an alias. It doesn't seem obvious to me what "oc" means at first glance.

Thanks, I had wondered if this is high profile enough to warrant an alias.

We can always add one later.

oscargus · 2022-06-07T09:17:30Z

The capstyle discussion also made me realize that it could be good to, once the basic case is handled, to change to test to use alpha, and multiple points in the plot, ideally making the two lines cross so that the effect of alpha (of one) can be evaluated.

(Multiple points as I cannot really realize what would happen when the lines change direction if there is a gap around there.)

rcomer · 2022-06-08T06:06:27Z

Multiple points as I cannot really realize what would happen when the lines change direction if there is a gap around there.

The new test image does show some overlaps between the red/black sections when they join on corners 😞

oscargus · 2022-06-08T06:28:09Z

when they join on corners

Ahh, that seems hard to solve... However, personally, I still think that there are cases where this is useful so it shouldn't be a showstopper. I am not sure how things like these are handled historically, but I would almost suggest that one adds some sort of "warning" in the docs that this may currently sometimes lead to unwanted effects when using alpha so one better check the results or something along those lines.

greglucas

I think this looks good and is a useful addition. There are some things that could be improved (caps, alpha-overlap), but I also think those could be quite challenging to handle robustly and can be implemented in follow-up PRs as improvements (I don't think they're worth blocking this over).

I think now you have the hard part left which is choosing the wording:

offcolor vs gapcolor vs something-else
stripey vs striped

lib/matplotlib/lines.py

greglucas · 2022-06-08T12:56:21Z

doc/users/next_whats_new/stripey_lines.rst

@@ -0,0 +1,19 @@
+Stripey Lines


I think "Striped" with a d sounds better, but that is just a personal preference. Might be regional dialect?

"Stripey" feels more natural to me but I don't have a strong opinion. Since the original PR and the issue used "striped" that's at least 3 votes to 1!

greglucas · 2022-06-08T13:16:24Z

lib/matplotlib/lines.py

+            In some cases the two line colors may overlap, which could lead to
+            unwanted effects if also setting ``alpha``.


I think this is a fact of the way this is implemented with two separate lines, so the renderer doesn't know they are meant to be the same object and so then it blends the transparencies.

Another option for the striped lines may be to do something similar to the multi-colored line example...
https://matplotlib.org/stable/gallery/lines_bars_and_markers/multicolored_line.html

The multicolored line is somewhat different from dashes because it assigns values/borders in data coordinates. I.e. you can zoom in to one color segment. With dashes however, we don't want the users to zoom in to a single dash. The dash size is independed of the data coordiantes.

Anyway, I've proposed to decleare this feature experimental so that we still have the freedom to come up with a different implementation.

timhoffm

Given that there could be issues with the rendering and we may want to change some things (e.g. maybe force a flat capstyle?), I suggest to declare this feature experimental so that we still have the feedom to change all aspects based on usage experience.

It would also be nice to mention this functionality in set_dashes.

lib/matplotlib/lines.py

timhoffm · 2022-06-10T07:21:24Z

lib/matplotlib/tests/test_lines.py

@@ -304,6 +304,17 @@ def test_marker_as_markerstyle():
    assert_array_equal(line3.get_marker().vertices, triangle1.vertices)


+@image_comparison(['striped_line.png'], remove_text=True, style='mpl20')


Interestingly, remove_text does not apply to legend. (Should be investigated at some point).

Still, I propose not to pick up text in the reference image. Do you want the legend entries as a test that the lines are drawn correctly there? If not remove the legend, if yes use label=' '.

@oscargus asked for the legend so I guess he wanted to check the legend lines are drawn correctly.

Correct! Completely happy without text!

lib/matplotlib/lines.py

rcomer · 2022-06-12T16:34:44Z

Hmmmm. The "Compare" on my latest force-push is showing a bunch of irrelevant changes. I don't know why that should be, as the overall change still looks fine.

timhoffm · 2022-06-12T21:06:39Z

doc/users/next_whats_new/striped_lines.rst

+    y = x**3
+
+    fig, ax = plt.subplots()
+    ax.plot(x, y, linestyle='--', color='orange', second_color='blue',


TL;DR I think we should go with gapcolor.

I'm very sorry for going back and forth, but I just realized how this parameter fits into the whole color story.

Seeing this makes me wonder whether second_color is actually a good name Because for

ax.plot(x, y, linestyle='o', color='orange', second_color='blue',

color affects the marker but second_color does not. This is a certain asymmetry.

Actually the color mechanism is somewhat complicated:

Visual part of Line2D artist explicit parameter Affected by color

marker face markerfacecolor x

additional marker face (for half-filled markers) markerfacecoloralt -

marker edge markeredgecolor x

line - x

this PR: additional line part (for dashed lines) second_color -

Observations:

color is a global parameter that can affect multiple visual parts. The behavior is reaonable: Color all visible parts with this color". (It's correct that this does not apply to the additional marker face of half-filled markers, because that would make them fully filled.)

Conceptually the gapcolor (I call it like this for now) is similar to markerfacecoloralt - it should and isn't affected by color.

We don't have an explicit linecolor parameter. That's also ok. Think: We start with a base color and one can optionally change all marker parameters on top of this.

When only thinking about the line part color + second_color is a perfectly good API. However, taking the whole picture into account, it's not specific enough. IMHO the name has to contain the visual part that is addressed. My suggestions in order of preference:

gapcolor

second_line_color (very clear, but very long).

linecoloralt (rhymes with markerfacecoloralt, but a bit awkward to have ...alt without a linecolor and I don't think we need to add that).

Way too late to change I know, but I don't think it would be bad to have a linecolor alias b/c in the marker+line case, is kinda confusing that the line color is color but the markercolor is markerfacecolor + markeredgecolor (also that there's no markercolor)

I think if we had second_line_color then a user might expect there to be a line_color. Someone (@efiring I think) mentioned that “alt color” has problematic meaning in some places.

So gapcolor also seems best to me.

jklymak · 2022-06-23T08:32:40Z

@timhoffm are you ok with this one?

timhoffm · 2022-06-23T21:28:22Z

@rcomer Thanks for picking this up! This is a really nice feature.

oscargus reviewed Jun 5, 2022

View reviewed changes

rcomer force-pushed the stripey-lines branch from 571837d to 9d4c639 Compare June 6, 2022 19:52

greglucas reviewed Jun 7, 2022

View reviewed changes

rcomer force-pushed the stripey-lines branch from 9d4c639 to 63fc233 Compare June 8, 2022 05:42

rcomer force-pushed the stripey-lines branch from 63fc233 to 069b436 Compare June 8, 2022 11:34

greglucas reviewed Jun 8, 2022

View reviewed changes

story645 mentioned this pull request Jun 8, 2022

Changed offset reference, add small doc #23223

Merged

rcomer force-pushed the stripey-lines branch 2 times, most recently from f153664 to 168535c Compare June 10, 2022 06:09

timhoffm reviewed Jun 10, 2022

View reviewed changes

rcomer force-pushed the stripey-lines branch from 6674d7b to 7dd887e Compare June 12, 2022 16:30

rcomer force-pushed the stripey-lines branch from 7dd887e to d540bee Compare June 12, 2022 16:40

oscargus approved these changes Jun 12, 2022

View reviewed changes

timhoffm reviewed Jun 12, 2022

View reviewed changes

rcomer force-pushed the stripey-lines branch from d540bee to 8e567ed Compare June 13, 2022 11:58

ENH: enable striped lines

83240b0

rcomer force-pushed the stripey-lines branch from 8e567ed to 83240b0 Compare June 13, 2022 12:00

jklymak requested a review from timhoffm June 23, 2022 08:32

timhoffm approved these changes Jun 23, 2022

View reviewed changes

timhoffm merged commit c19ffdf into matplotlib:main Jun 23, 2022

QuLogic added this to the v3.6.0 milestone Jun 23, 2022

rcomer deleted the stripey-lines branch June 24, 2022 19:25

rcomer mentioned this pull request Dec 23, 2022

[Bug]: gapcolor not supported for LineCollections #24796

Closed

story645 mentioned this pull request Jan 16, 2023

[MNT]: _check_color_like for list like input #25005

Closed

rcomer mentioned this pull request Feb 3, 2023

Stripey LineCollection #24849

Merged

5 tasks

rcomer added the New feature label Jul 3, 2023

rcomer mentioned this pull request Nov 9, 2023

[Tag]: version: 3.N #27292

Open

story645 mentioned this pull request Apr 10, 2024

document internal mechanics of dash scaling #7837

Open

		@@ -312,14 +315,12 @@ def __init__(self, xdata, ydata,

		if linewidth is None:
		linewidth = rcParams['lines.linewidth']

		In some cases the two line colors may overlap, which could lead to
		unwanted effects if also setting ``alpha``.

		@@ -304,6 +304,17 @@ def test_marker_as_markerstyle():
		assert_array_equal(line3.get_marker().vertices, triangle1.vertices)


		@image_comparison(['striped_line.png'], remove_text=True, style='mpl20')

Visual part of Line2D artist	explicit parameter	Affected by `color`
marker face	markerfacecolor	x
additional marker face (for half-filled markers)	markerfacecoloralt	-
marker edge	markeredgecolor	x
line	-	x
this PR: additional line part (for dashed lines)	second_color	-

ENH: enable stripey lines #23208

ENH: enable stripey lines #23208

Conversation

rcomer commented Jun 5, 2022 • edited Loading

PR Summary

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscargus commented Jun 5, 2022 • edited Loading

oscargus commented Jun 5, 2022

rcomer commented Jun 5, 2022 • edited Loading

oscargus commented Jun 6, 2022

oscargus commented Jun 6, 2022

rcomer commented Jun 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

story645 Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscargus commented Jun 7, 2022 • edited Loading

rcomer commented Jun 8, 2022

oscargus commented Jun 8, 2022 • edited Loading

greglucas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timhoffm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcomer commented Jun 12, 2022

timhoffm Jun 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklymak commented Jun 23, 2022

timhoffm commented Jun 23, 2022

rcomer commented Jun 5, 2022 •

edited

Loading

oscargus commented Jun 5, 2022 •

edited

Loading

rcomer commented Jun 5, 2022 •

edited

Loading

story645 Jun 8, 2022 •

edited

Loading

oscargus commented Jun 7, 2022 •

edited

Loading

oscargus commented Jun 8, 2022 •

edited

Loading

timhoffm Jun 12, 2022 •

edited

Loading