Skip to content

Simplify _bind_draw_path_function. #23571

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
Aug 19, 2022
Merged

Simplify _bind_draw_path_function. #23571

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
70 changes: 32 additions & 38 deletions lib/matplotlib/patches.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
Patches are `.Artist`\s with a face color and an edge color.
"""

import contextlib
import functools
import inspect
import math
Expand Down Expand Up @@ -529,15 +528,15 @@ def get_hatch(self):
"""Return the hatching pattern."""
return self._hatch

@contextlib.contextmanager
def _bind_draw_path_function(self, renderer):
def _draw_paths_with_artist_properties(
self, renderer, draw_path_args_list):
"""
``draw()`` helper factored out for sharing with `FancyArrowPatch`.

Yields a callable ``dp`` such that calling ``dp(*args, **kwargs)`` is
equivalent to calling ``renderer1.draw_path(gc, *args, **kwargs)``
where ``renderer1`` and ``gc`` have been suitably set from ``renderer``
and the artist's properties.
Configure *renderer* and the associated graphics context *gc*
from the artist properties, then repeatedly call
``renderer.draw_path(gc, *draw_path_args)`` for each tuple
*draw_path_args* in *draw_path_args_list*.
"""

renderer.open_group('patch', self.get_gid())
Expand Down Expand Up @@ -571,11 +570,8 @@ def _bind_draw_path_function(self, renderer):
from matplotlib.patheffects import PathEffectRenderer
renderer = PathEffectRenderer(self.get_path_effects(), renderer)

# In `with _bind_draw_path_function(renderer) as draw_path: ...`
# (in the implementations of `draw()` below), calls to `draw_path(...)`
# will occur as if they took place here with `gc` inserted as
# additional first argument.
yield functools.partial(renderer.draw_path, gc)
for draw_path_args in draw_path_args_list:
renderer.draw_path(gc, *draw_path_args)

gc.restore()
renderer.close_group('patch')
Expand All @@ -586,16 +582,17 @@ def draw(self, renderer):
# docstring inherited
if not self.get_visible():
return
with self._bind_draw_path_function(renderer) as draw_path:
path = self.get_path()
transform = self.get_transform()
tpath = transform.transform_path_non_affine(path)
affine = transform.get_affine()
draw_path(tpath, affine,
# Work around a bug in the PDF and SVG renderers, which
# do not draw the hatches if the facecolor is fully
# transparent, but do if it is None.
self._facecolor if self._facecolor[3] else None)
path = self.get_path()
transform = self.get_transform()
tpath = transform.transform_path_non_affine(path)
affine = transform.get_affine()
self._draw_paths_with_artist_properties(
renderer,
[(tpath, affine,
# Work around a bug in the PDF and SVG renderers, which
# do not draw the hatches if the facecolor is fully
# transparent, but do if it is None.
self._facecolor if self._facecolor[3] else None)])
Comment on lines +585 to +595
Copy link
Member

Choose a reason for hiding this comment

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

Optional: What do you think of introducing

_DrawPathArgs = namedtuple('_DrawPathArgs', 'path, transform, rgb_face')

The main motivation is to name the tuple elements because the function signature
where we could look up their meaning is quite far away.

With this we could write:

Suggested change
path = self.get_path()
transform = self.get_transform()
tpath = transform.transform_path_non_affine(path)
affine = transform.get_affine()
self._draw_paths_with_artist_properties(
renderer,
[(tpath, affine,
# Work around a bug in the PDF and SVG renderers, which
# do not draw the hatches if the facecolor is fully
# transparent, but do if it is None.
self._facecolor if self._facecolor[3] else None)])
transform = self.get_transform()
self._draw_paths_with_artist_properties(
renderer,
[_DrawPathArgs(
path=transform.transform_path_non_affine(self.get_path),
transform=transform.get_affine(),
# Work around a bug in the PDF and SVG renderers, which
# do not draw the hatches if the facecolor is fully
# transparent, but do if it is None.
rgb_face=self._facecolor if self._facecolor[3] else None),
])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched to using a dict (and **kwargs unpacking), which seems just as good here.


def get_path(self):
"""Return the path of this patch."""
Expand Down Expand Up @@ -4415,25 +4412,22 @@ def draw(self, renderer):
if not self.get_visible():
return

with self._bind_draw_path_function(renderer) as draw_path:

# FIXME : dpi_cor is for the dpi-dependency of the linewidth. There
# could be room for improvement. Maybe _get_path_in_displaycoord
# could take a renderer argument, but get_path should be adapted
# too.
self._dpi_cor = renderer.points_to_pixels(1.)
path, fillable = self._get_path_in_displaycoord()
# FIXME: dpi_cor is for the dpi-dependency of the linewidth. There
# could be room for improvement. Maybe _get_path_in_displaycoord could
# take a renderer argument, but get_path should be adapted too.
self._dpi_cor = renderer.points_to_pixels(1.)
Copy link
Member

Choose a reason for hiding this comment

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

So its OK now for self._dpi_cor to be changed permanently here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was already set permanently before?

path, fillable = self._get_path_in_displaycoord()

if not np.iterable(fillable):
path = [path]
fillable = [fillable]
if not np.iterable(fillable):
path = [path]
fillable = [fillable]

affine = transforms.IdentityTransform()
affine = transforms.IdentityTransform()

for p, f in zip(path, fillable):
draw_path(
p, affine,
self._facecolor if f and self._facecolor[3] else None)
self._draw_paths_with_artist_properties(
renderer,
[(p, affine, self._facecolor if f and self._facecolor[3] else None)
for p, f in zip(path, fillable)])


class ConnectionPatch(FancyArrowPatch):
Expand Down