Skip to content

MarkerStyle is considered immutable #19341

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 28, 2021
Merged

MarkerStyle is considered immutable #19341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/api/next_api_changes/deprecations/19341-TH.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
``MarkerStyle`` is considered immutable
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``MarkerStyle.set_fillstyle()`` and ``MarkerStyle.set_marker()`` are
deprecated. Create a new ``MarkerStyle`` with the respective parameters
instead.
9 changes: 5 additions & 4 deletions lib/matplotlib/lines.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ def set_fillstyle(self, fs):

For examples see :ref:`marker_fill_styles`.
"""
self._marker.set_fillstyle(fs)
self.set_marker(MarkerStyle(self._marker.get_marker(), fs))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we never test set_fillstyle? Can we use this as the opportunity to do so?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have this deprecation be part of 3.4 and I don't know if I have the bandwidth to add tests soon. Therefore, I'd rather do this in a later PR.

self.stale = True

def set_markevery(self, every):
Expand Down Expand Up @@ -930,7 +930,8 @@ def get_markeredgecolor(self):
if rcParams['_internal.classic_mode']:
if self._marker.get_marker() in ('.', ','):
return self._color
if self._marker.is_filled() and self.get_fillstyle() != 'none':
if (self._marker.is_filled()
and self._marker.get_fillstyle() != 'none'):
return 'k' # Bad hard-wired default...
return self._color
else:
Expand All @@ -945,7 +946,7 @@ def get_markeredgewidth(self):
return self._markeredgewidth

def _get_markerfacecolor(self, alt=False):
if self.get_fillstyle() == 'none':
if self._marker.get_fillstyle() == 'none':
return 'none'
fc = self._markerfacecoloralt if alt else self._markerfacecolor
if cbook._str_lower_equal(fc, 'auto'):
Expand Down Expand Up @@ -1166,7 +1167,7 @@ def set_marker(self, marker):
See `~matplotlib.markers` for full description of possible
arguments.
"""
self._marker.set_marker(marker)
self._marker = MarkerStyle(marker, self._marker.get_fillstyle())
self.stale = True

def set_markeredgecolor(self, ec):
Expand Down
15 changes: 13 additions & 2 deletions lib/matplotlib/markers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ class MarkerStyle:
"""
A class representing marker types.

Instances are immutable. If you need to change anything, create a new
instance.

Attributes
----------
markers : list
Expand Down Expand Up @@ -228,8 +231,8 @@ def __init__(self, marker=None, fillstyle=None):
One of 'full', 'left', 'right', 'bottom', 'top', 'none'.
"""
self._marker_function = None
self.set_fillstyle(fillstyle)
self.set_marker(marker)
self._set_fillstyle(fillstyle)
self._set_marker(marker)

def _recache(self):
if self._marker_function is None:
Expand All @@ -256,7 +259,11 @@ def is_filled(self):
def get_fillstyle(self):
return self._fillstyle

@_api.deprecated("3.4", alternative="a new marker")
def set_fillstyle(self, fillstyle):
return self._set_fillstyle(fillstyle)

def _set_fillstyle(self, fillstyle):
"""
Set the fillstyle.

Expand All @@ -281,7 +288,11 @@ def get_capstyle(self):
def get_marker(self):
return self._marker

@_api.deprecated("3.4", alternative="a new marker")
def set_marker(self, marker):
return self._set_marker(marker)

def _set_marker(self, marker):
"""
Set the marker.

Expand Down
50 changes: 33 additions & 17 deletions lib/matplotlib/tests/test_marker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,43 @@ def test_marker_fillstyle():
assert not marker_style.is_filled()


def test_markers_valid():
marker_style = markers.MarkerStyle()
mrk_array = np.array([[-0.5, 0],
[0.5, 0]])
@pytest.mark.parametrize('marker', [
'o',
'x',
'',
'None',
None,
r'$\frac{1}{2}$',
"$\u266B$",
1,
markers.TICKLEFT,
[[-1, 0], [1, 0]],
np.array([[-1, 0], [1, 0]]),
Path([[0, 0], [1, 0]], [Path.MOVETO, Path.LINETO]),
(5, 0), # a pentagon
(7, 1), # a 7-pointed star
(5, 2), # asterisk
(5, 0, 10), # a pentagon, rotated by 10 degrees
(7, 1, 10), # a 7-pointed star, rotated by 10 degrees
(5, 2, 10), # asterisk, rotated by 10 degrees
markers.MarkerStyle(),
markers.MarkerStyle('o'),
])
def test_markers_valid(marker):
# Checking this doesn't fail.
marker_style.set_marker(mrk_array)
markers.MarkerStyle(marker)


def test_markers_invalid():
marker_style = markers.MarkerStyle()
mrk_array = np.array([[-0.5, 0, 1, 2, 3]])
# Checking this does fail.
@pytest.mark.parametrize('marker', [
'square', # arbitrary string
np.array([[-0.5, 0, 1, 2, 3]]), # 1D array
(1,),
(5, 3), # second parameter of tuple must be 0, 1, or 2
(1, 2, 3, 4),
])
def test_markers_invalid(marker):
with pytest.raises(ValueError):
marker_style.set_marker(mrk_array)


def test_marker_path():
marker_style = markers.MarkerStyle()
path = Path([[0, 0], [1, 0]], [Path.MOVETO, Path.LINETO])
# Checking this doesn't fail.
marker_style.set_marker(path)
markers.MarkerStyle(marker)


class UnsnappedMarkerStyle(markers.MarkerStyle):
Expand Down