Skip to content

Cleanup list copying #16327

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 30, 2020
Merged

Cleanup list copying #16327

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
2 changes: 1 addition & 1 deletion doc/sphinxext/missing_references.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ def prepare_missing_references_handler(app):

sphinx_logger = logging.getLogger('sphinx')
missing_reference_filter = MissingReferenceFilter(app)
for handler in sphinx_logger.handlers[:]:
for handler in sphinx_logger.handlers:
if (isinstance(handler, sphinx_logging.WarningStreamHandler)
and missing_reference_filter not in handler.filters):

Expand Down
4 changes: 2 additions & 2 deletions lib/matplotlib/artist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1065,8 +1065,8 @@ def update_from(self, other):
self._label = other._label
self._sketch = other._sketch
self._path_effects = other._path_effects
self.sticky_edges.x[:] = other.sticky_edges.x[:]
self.sticky_edges.y[:] = other.sticky_edges.y[:]
self.sticky_edges.x[:] = other.sticky_edges.x.copy()
self.sticky_edges.y[:] = other.sticky_edges.y.copy()
self.pchanged()
self.stale = True

Expand Down
5 changes: 4 additions & 1 deletion lib/matplotlib/backend_bases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ def draw_gouraud_triangle(self, gc, points, colors, transform):
Parameters
----------
gc : `.GraphicsContextBase`
The graphics context.
points : array-like, shape=(3, 2)
Array of (x, y) points for the triangle.
Expand Down Expand Up @@ -1059,7 +1062,7 @@ def __init__(self, interval=None, callbacks=None):
if callbacks is None:
self.callbacks = []
else:
self.callbacks = callbacks[:] # Create a copy
self.callbacks = callbacks.copy()

if interval is None:
self._interval = 1000
Expand Down
17 changes: 10 additions & 7 deletions lib/matplotlib/backends/backend_svg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,8 @@ def draw_path_collection(self, gc, master_transform, paths, all_transforms,
self._path_collection_id += 1

def draw_gouraud_triangle(self, gc, points, colors, trans):
# docstring inherited

# This uses a method described here:
#
# http://www.svgopen.org/2005/papers/Converting3DFaceToSVG/index.html
Expand Down Expand Up @@ -685,9 +687,9 @@ def draw_gouraud_triangle(self, gc, points, colors, trans):
' \n1 1 1 1 0 \n0 0 0 0 1 ')
writer.end('filter')

avg_color = np.sum(colors[:, :], axis=0) / 3.0
# Just skip fully-transparent triangles
if avg_color[-1] == 0.0:
avg_color = np.average(colors, axis=0)
if avg_color[-1] == 0:
# Skip fully-transparent triangles
return

trans_and_flip = self._make_flip_transform(trans)
Expand All @@ -698,7 +700,7 @@ def draw_gouraud_triangle(self, gc, points, colors, trans):
x1, y1 = tpoints[i]
x2, y2 = tpoints[(i + 1) % 3]
x3, y3 = tpoints[(i + 2) % 3]
c = colors[i][:]
rgba_color = colors[i]

if x2 == x3:
xb = x2
Expand All @@ -723,12 +725,13 @@ def draw_gouraud_triangle(self, gc, points, colors, trans):
writer.element(
'stop',
offset='1',
style=generate_css({'stop-color': rgb2hex(avg_color),
'stop-opacity': short_float_fmt(c[-1])}))
style=generate_css({
'stop-color': rgb2hex(avg_color),
'stop-opacity': short_float_fmt(rgba_color[-1])}))
writer.element(
'stop',
offset='0',
style=generate_css({'stop-color': rgb2hex(c),
style=generate_css({'stop-color': rgb2hex(rgba_color),
'stop-opacity': "0"}))

writer.end('linearGradient')
Expand Down
7 changes: 2 additions & 5 deletions lib/matplotlib/patches.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3159,11 +3159,8 @@ def __call__(self, path, mutation_size, linewidth,

if aspect_ratio is not None:
# Squeeze the given height by the aspect_ratio

vertices, codes = path.vertices[:], path.codes[:]
Copy link
Contributor

@anntzer anntzer Jan 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

path_shrunk = Path(path.vertices / [1, aspect_ratio], path.codes)?
(Actually the old code looks wrong -- it would modify path's vertices in places too)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. But I think I still need to copy the codes.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, because path codes (and vertices) are explicitly immutable

    .. note::

        The vertices and codes arrays should be treated as
        immutable -- there are a number of optimizations and assumptions
        made up front in the constructor that will not change when the
        data changes.

so it's fine to reuse the same array.

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 read this the other way round: "Don't modify vertices and codes." We assume that they stay the same, but it's not checked.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not checked but what happens if someone changes it is explicitly undefined behavior, so we can assume that they are not changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

IMHO this claims that users are not allowed to change the code. But it does not guarantee that Path does not modify the input. In fact, if codes is a numpy array already, it's writable flag is set to False.

Anyway, I don't want to go into a discussion about who owns the data. In this place, it's just reusing codes from another path, so nothing bad should happen anyway. I've removed the copy, which is consistent with the current behavior (the slice codes[:] also shares the data).

# Squeeze the height
vertices[:, 1] = vertices[:, 1] / aspect_ratio
path_shrunk = Path(vertices, codes)
vertices = path.vertices / [1, aspect_ratio]
path_shrunk = Path(vertices, path.codes)
# call transmute method with squeezed height.
path_mutated, fillable = self.transmute(path_shrunk,
linewidth,
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/widgets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2578,7 +2578,7 @@ def _press(self, event):
self._active_handle_idx = h_idx
# Save the vertex positions at the time of the press event (needed to
# support the 'move_all' state modifier).
self._xs_at_press, self._ys_at_press = self._xs[:], self._ys[:]
self._xs_at_press, self._ys_at_press = self._xs.copy(), self._ys.copy()

def _release(self, event):
"""Button release event handler"""
Expand Down