FIX: refactor dash scaling #6710
Conversation
| import matplotlib.path as mpath | ||
| from matplotlib import _path | ||
| import matplotlib.mlab as mlab | ||
| import matplotlib.lines as mlines | ||
|
|
||
| import math |
There was a problem hiding this comment.
Should go at the beginning, really. And not remove the second blank line.
aeda340 to
abee361
Compare
|
Closes #6693
from @afvincent in the issue import matplotlib as mpl
import matplotlib.pyplot as plt
from matplotlib.ticker import NullLocator
def plot_test(ax, ls=None, lw=None):
_plot_opts = dict(facecolor='LightGrey', edgecolor='Black')
ax.set_title("ls:'{ls}' & lw: {lw}".format(ls=ls, lw=lw))
ax.fill_between([1, 2, 3], [4, 5, 6], 5, linestyle=ls, lw=lw, **_plot_opts)
ax.xaxis.set_major_locator(NullLocator())
ax.yaxis.set_major_locator(NullLocator())
return ax
fig, ((ax0, ax1), (ax2, ax3)) = plt.subplots(nrows=2, ncols=2,
num=mpl.__version__)
ax0 = plot_test(ax0, ls='dashed', lw=2)
ax1 = plot_test(ax1, ls='dashed', lw=10)
ax2 = plot_test(ax2, ls='dashed', lw=None)
ax3 = plot_test(ax3, ls='solid', lw=0)and import matplotlib.pyplot as plt
plt.fill_between([1,2,3], [4,5,6], 5, linestyle="None") |
|
Also closes #5430 |
| 'Do not know how to convert {!r} to dashes'.format(ls)) | ||
|
|
||
| self._us_linestyles = dashes | ||
| self._linewidths |
There was a problem hiding this comment.
I guess the line just above is a typo, isn't it?
There was a problem hiding this comment.
Fixed. Left over from me spell checking variables 🐑
abee361 to
c18140c
Compare
|
I think the dash scaling needs to happen from both set_linestyle and set_linewidth. Otherwise, this looks good. |
| elif isinstance(style, tuple): | ||
| offset, dashes = style | ||
| else: | ||
| raise ValueError('Unrecognized linestyle: %s' % str(style)) | ||
|
|
||
| # normalize offset to be positive and shorter than the dash cycle | ||
| if dashes is not None and offset is not None: | ||
| offset %= sum(dashes) |
There was a problem hiding this comment.
Don't know if one are supposed to be “bad user-proof”, but if dashes is (0, 0), won't it raise a ZeroDivisionError? (I know it's kind of a corner case though)
|
Sorry -- I just missed it. The context in the diff wasn't so clear. LGTM. |
| @@ -117,7 +122,10 @@ def __init__(self, | |||
| """ | |||
| artist.Artist.__init__(self) | |||
| cm.ScalarMappable.__init__(self, norm, cmap) | |||
|
|
|||
| self._us_linestyles = [(None, None)] | |||
There was a problem hiding this comment.
FWIW, it was not obvious to me that “_us” prefix stood for “unscaled” (at least until I read ~ l. 390 in lines.py). Adding a small comment might be useful for future readers.
|
@tacaswell : Would it be possible to write a small docstring for the static method |
ec4f168 to
f67150a
Compare
|
@afvincent Thanks! |
|
You're welcome (but you're the one who did the hard work). For my knowledge, about ll.606-609 in # pass in None as first arg as collection do not
# support offset in linestyles (yet)
dashes = [mlines._scale_dashes(o, d, lw)
for (o, d), lw in zip(dashes, linewidths)]where is |
When scaling the dash pattern by the linewidth do the scaling at artist creation / value set time rather than at draw time. closes matplotlib#6592 closes matplotlib#6588 closes matplotlib#6590 Closes matplotlib#6693 closes matplotlib#5430
f67150a to
3ef3f5b
Compare
|
That is an old comment (it was a windy road to sort out how to make this work) that is no longer true. |
This start to address #6592
The collection code still is broken.