FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

ENH: Allow for non-normalized and transformed markers by ImportanceOfBeingErnest · Pull Request #16773 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

ENH: Allow for non-normalized and transformed markers #16773

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

Closed
ImportanceOfBeingErnest wants to merge 1 commit into from
Conversation 52 Commits 1 Checks 0 Files changed
Closed

ENH: Allow for non-normalized and transformed markers #16773

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 .py  (3) .rst  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
12 changes: 12 additions & 0 deletions doc/users/next_whats_new/2020-03-16_markerstyle_normalization.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Allow for custom marker scaling
-------------------------------
`~.markers.MarkerStyle` gained a keyword argument *normalization*, which may be
set to *"none"* to allow for custom paths to not be scaled.::

MarkerStyle(Path(...), normalization="none")

`~.markers.MarkerStyle` also gained a `~.markers.MarkerStyle.set_transform`
method to set affine transformations to existing markers.::

m = MarkerStyle("d")
m.set_transform(m.get_transform() + Affine2D().rotate_deg(30))
65 changes: 58 additions & 7 deletions examples/lines_bars_and_markers/scatter_piecharts.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,19 @@
Scatter plot with pie chart markers
===================================

This example makes custom 'pie charts' as the markers for a scatter plot.

Thanks to Manuel Metz for the example.
This example shows two methods to make custom 'pie charts' as the markers
for a scatter plot.
"""

##########################################################################
# Manually creating marker vertices
# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#

import numpy as np
import matplotlib.pyplot as plt

# first define the ratios
# first define the cumulative ratios
r1 = 0.2 # 20%
r2 = r1 + 0.4 # 40%

Expand All @@ -36,10 +40,55 @@
s3 = np.abs(xy3).max()

fig, ax = plt.subplots()
ax.scatter(range(3), range(3), marker=xy1, s=s1**2 * sizes, facecolor='blue')
ax.scatter(range(3), range(3), marker=xy2, s=s2**2 * sizes, facecolor='green')
ax.scatter(range(3), range(3), marker=xy3, s=s3**2 * sizes, facecolor='red')
ax.scatter(range(3), range(3), marker=xy1, s=s1**2 * sizes, facecolor='C0')
ax.scatter(range(3), range(3), marker=xy2, s=s2**2 * sizes, facecolor='C1')
ax.scatter(range(3), range(3), marker=xy3, s=s3**2 * sizes, facecolor='C2')

plt.show()


##########################################################################
# Using wedges as markers
# ~~~~~~~~~~~~~~~~~~~~~~~
#
# An alternative is to create custom markers from the `~.path.Path` of a
# `~.patches.Wedge`, which might be more versatile.
#

import numpy as np
import matplotlib.pyplot as plt
from matplotlib.patches import Wedge
from matplotlib.markers import MarkerStyle

# first define the ratios
r1 = 0.2 # 20%
r2 = r1 + 0.3 # 50%
r3 = 1 - r1 - r2 # 30%


def markers_from_ratios(ratios, width=1):
markers = []
angles = 360*np.concatenate(([0], np.cumsum(ratios)))
for i in range(len(angles)-1):
# create a Wedge within the unit square in between the given angles...
w = Wedge((0, 0), 0.5, angles[i], angles[i+1], width=width/2)
# ... and create a custom Marker from its path.
markers.append(MarkerStyle(w.get_path(), normalization="none"))
return markers

# define some sizes of the scatter marker
sizes = np.array([100, 200, 400, 800])
# collect the markers and some colors
markers = markers_from_ratios([r1, r2, r3], width=0.6)
colors = plt.cm.tab10.colors[:len(markers)]

fig, ax = plt.subplots()

for marker, color in zip(markers, colors):
ax.scatter(range(len(sizes)), range(len(sizes)), marker=marker, s=sizes,
edgecolor="none", facecolor=color)

ax.margins(0.1)
plt.show()

#############################################################################
Expand All @@ -55,3 +104,5 @@
import matplotlib
matplotlib.axes.Axes.scatter
matplotlib.pyplot.scatter
matplotlib.patches.Wedge
matplotlib.markers.MarkerStyle
30 changes: 24 additions & 6 deletions lib/matplotlib/markers.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ class MarkerStyle:
# TODO: Is this ever used as a non-constant?
_point_size_reduction = 0.5

def __init__(self, marker=None, fillstyle=None):
def __init__(self, marker=None, fillstyle=None, *,
normalization="classic"):

jklymak Mar 23, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

is there a reason this can't be a transform= kwarg? ie transform=None would do what we did before...
"""
Attributes
----------
Expand All @@ -213,12 +214,23 @@ def __init__(self, marker=None, fillstyle=None):

Parameters
----------
marker : str or array-like, optional, default: None
marker : str, array-like, `~.path.Path`, or `~.markers.MarkerStyle`, \
default: None
Comment thread
ImportanceOfBeingErnest marked this conversation as resolved.
See the descriptions of possible markers in the module docstring.

fillstyle : str, optional, default: 'full'
'full', 'left", 'right', 'bottom', 'top', 'none'

normalization : str, {'classic', 'none'}, optional, default: "classic"
The normalization of the marker size. Only applies to custom paths
that are provided as array of vertices or `~.path.Path`.
Can take two values:
*'classic'*, being the default, makes sure the marker path is

QuLogic Mar 23, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

classic is a confusing name, because it overlaps with the classic style, but it's not the classic style (I mean it is also the classic style, but not only the classic style.)

brunobeltran Mar 31, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I might be misreading this comment, but it seems like it's not that classic is a confusing name, but that the docstring here is not correct.

According to the discussion above. The docstring should read (if I understand correctly):

*'classic'*, being the default, ensures built-in markers maintain the size they have always had, and for custom markers it makes sure the marker path is normalized to fit within a unit-square by affine scaling.

This is exactly the classic style, as far as I can tell.

ImportanceOfBeingErnest Mar 31, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hence the sentence "Only applies to custom paths...". So this is definitely correct.

I'm happy to use a name other than "classic", but since I find that name fits pretty well, I would need input with alternative ideas. I guess this is also secondary compared to naming the argument itself - for which there is also no consensus yet it seems.

brunobeltran Mar 31, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

To be clear, I am 100% behind the "classic" name. I think it is consistently applied and fits well.

I am also very happy with "normalization".

However, wasn't one of the main benefits (and the impetus for this PR being revisited) that adding a "normalization" kwarg would allow us to address #15703? If so, I think that stating in the initial docstring that it only applies to custom paths (maybe just add "for now" if you don't like my more verbose wording).

ImportanceOfBeingErnest Mar 31, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm cautious to document any possible future behaviour. (We had some new normalization or the toolmanager being anounced as "coming soon" for years, which doesn't really help people using the library in the present.)
While it's important to think ahead and make sure to not block any roads (which is why we're having this discussion now), what's documented is the actual feature.

brunobeltran Mar 31, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

100% onboard with not documenting "possible" future behavior. On the other hand, a wording closer to mine doesn't close the door to this feature (which is already implemented in my "paths" PR stack).

Mostly just being selfish because I have a strong feeling that if the current phrasing of the docstring gets merged, I'll get pushback when I try to change it in my own PR, whereas an equally specific and correct version could be written that doesn't specify that this keyword "only applies to custom paths".

As far as I'm concerned, feel free to merge this as-is and I'll just have that fight when it's time.
normalized to fit within a unit-square by affine scaling.
*'none'*, in which case no scaling is performed on the marker path.
"""
cbook._check_in_list(["classic", "none"], normalization=normalization)
self._normalize = normalization
self._marker_function = None
self.set_fillstyle(fillstyle)
self.set_marker(marker)
Expand Down Expand Up @@ -303,6 +315,13 @@ def get_path(self):
def get_transform(self):
return self._transform.frozen()

def set_transform(self, transform):
"""
Sets the transform of the marker. This is the transform by which the

QuLogic Mar 23, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

First line of docstring should be separate.

Suggested change
Sets the transform of the marker. This is the transform by which the
Sets the transform of the marker.
This is the transform by which the marker path is transformed.

marker path is transformed.
"""
self._transform = transform

def get_alt_path(self):
return self._alt_path

Expand All @@ -316,8 +335,9 @@ def _set_nothing(self):
self._filled = False

def _set_custom_marker(self, path):
rescale = np.max(np.abs(path.vertices)) # max of x's and y's.
self._transform = Affine2D().scale(0.5 / rescale)
if self._normalize == "classic":
rescale = np.max(np.abs(path.vertices)) # max of x's and y's.
self._transform = Affine2D().scale(0.5 / rescale)
self._path = path

def _set_path_marker(self):
Expand Down Expand Up @@ -350,8 +370,6 @@ def _set_tuple_marker(self):
def _set_mathtext_path(self):
"""
Draws mathtext markers '$...$' using TextPath object.

Submitted by tcb
"""
from matplotlib.text import TextPath

Expand Down
24 changes: 23 additions & 1 deletion lib/matplotlib/tests/test_marker.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
import matplotlib.pyplot as plt
from matplotlib import markers
from matplotlib.path import Path
from matplotlib.testing.decorators import check_figures_equal
from matplotlib.transforms import Affine2D

from matplotlib.testing.decorators import check_figures_equal
import pytest


Expand Down Expand Up @@ -133,3 +134,24 @@ def draw_ref_marker(y, style, size):

ax_test.set(xlim=(-0.5, 1.5), ylim=(-0.5, 1.5))
ax_ref.set(xlim=(-0.5, 1.5), ylim=(-0.5, 1.5))


@check_figures_equal(extensions=["png"])
def test_marker_normalization(fig_test, fig_ref):
plt.style.use("mpl20")

ax = fig_ref.subplots()
ax.margins(0.3)
ax.scatter([0, 1], [0, 0], s=400, marker="s", c="C2")

ax = fig_test.subplots()
ax.margins(0.3)
# test normalize
p = Path([[0, 0], [1, 0], [1, 1], [0, 1], [0, 0]], closed=True)
p1 = p.transformed(Affine2D().translate(-.5, -.5).scale(20))
m1 = markers.MarkerStyle(p1, normalization="none")
ax.scatter([0], [0], s=1, marker=m1, c="C2")
# test transform
m2 = markers.MarkerStyle("s")
m2.set_transform(m2.get_transform() + Affine2D().scale(20))
ax.scatter([1], [0], s=1, marker=m2, c="C2")
Back | FazBrowse Home | New Git URL