Fix #5917. New dash patterns. Scale dashes by lw #5926

Merged
merged 2 commits into from Feb 8, 2016

Conversation

Projects
None yet
6 participants
Owner

mdboom commented Jan 27, 2016

This changes the dash patterns as suggested by @njsmith in #5917.

The "standard" 3 dash patterns are no longer hardcoded, but are not settable from rcParams.

Dash patterns are now scaled by the linewidth, unless running in "classic compatibility mode". As suggested by #5917 (comment), scaling by linewidth is not applied when the linewidth gets small. I'm just doing scale = max(1, linewidth) as a function there, but we could be more clever if we wanted to. The following shows the scaling by linewidth.

figure_1-1

  • The macosx backend will need special updates to support scaling by linewidth, which I think is best done after a first round of comments here.

mdboom added the needs_review label Jan 27, 2016

Contributor

dopplershift commented Jan 27, 2016

👍 from me. This is another nice gain in the aesthetics. Nothing jumped out at me in the implementation.

Member

WeatherGod commented Jan 27, 2016

One concern is with the legend. In linewidth 5 image, the dashed and dashdot lines are cut off in such a way that you don't see the complete sequence. Is there some way to guarantee that at least one complete cycle is drawn?

Owner

mdboom commented Jan 27, 2016

One concern is with the legend. In linewidth 5 image, the dashed and dashdot lines are cut off in such a way that you don't see the complete sequence. Is there some way to guarantee that at least one complete cycle is drawn?

Yeah, I saw that, too. On the one hand, a line width of 5 is somewhat outside of the norm, so I'm not too concerned (and if you want big lines, you probably want to just increase the dpi).

We could look into making the legend swatches longer -- but making sure it's always long enough for whatever random dash pattern the user gives will be tricky.

mdboom added some commits Jan 27, 2016

@mdboom mdboom Fix #5917. New dash patterns. Scale dashes by lw
cb52bc0
@mdboom mdboom Fix short dash style aliases
104e98e

@efiring efiring added a commit that referenced this pull request Feb 8, 2016

@efiring efiring Merge pull request #5926 from mdboom/new-dash-styles
Fix #5917. New dash patterns. Scale dashes by lw
eb2b77b

@efiring efiring merged commit eb2b77b into matplotlib:master Feb 8, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

efiring removed the needs_review label Feb 8, 2016

@tacaswell tacaswell added a commit that referenced this pull request Feb 8, 2016

@efiring @tacaswell efiring + tacaswell Merge pull request #5926 from mdboom/new-dash-styles
Fix #5917. New dash patterns. Scale dashes by lw
069785d
Owner

tacaswell commented Feb 8, 2016

backported to v2.x as 069785d

Owner

mdboom commented Feb 9, 2016

I think this got merged too soon. This doesn't include the fix for the macosx backend to scale dashes by linewidth. I've made a new issue in #5986

Owner

efiring commented Feb 9, 2016

Mike, this was deliberate. Tom and I decided it was better not to hold up the style changes for the sake of the macosx backend. If the necessary modifications to that backend can be made, so much the better, but in the meantime we need to move forward to get 2.0 out, with or without full macosx backend compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment