Skip to content

fixes issue #2556 #2558

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 8 commits into from
Jan 27, 2014
Merged

fixes issue #2556 #2558

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1b81e9f
fixes issue #2556
tacaswell Oct 29, 2013
fcaf629
pep8 fix
tacaswell Oct 31, 2013
5e69629
renamed import
tacaswell Jan 13, 2014
c312cca
added test_quiver module
tacaswell Jan 14, 2014
4cc466e
fixed memory leak in quiverkey
tacaswell Jan 14, 2014
e0972f0
pep8 + minor variable re-naming
tacaswell Jan 21, 2014
c07f674
removed un-needed weakref
tacaswell Jan 21, 2014
f92ee59
added missing self
tacaswell Jan 22, 2014
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
Prev Previous commit
Next Next commit
fixed memory leak in quiverkey 
closes #2556
  • Loading branch information
@tacaswell
tacaswell committed Jan 14, 2014
commit 4cc466e5bb60b8c2751908d0e9dfb84d04773345
37 changes: 29 additions & 8 deletions lib/matplotlib/quiver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,9 @@

class QuiverKey(martist.Artist):
""" Labelled arrow for use as a quiver plot scale key."""
halign = {'N': 'center', 'S': 'center', 'E': 'left', 'W': 'right'}
valign = {'N': 'bottom', 'S': 'top', 'E': 'center', 'W': 'center'}
pivot = {'N': 'mid', 'S': 'mid', 'E': 'tip', 'W': 'tail'}
halign = {'N': 'center', 'S': 'center', 'E': 'left', 'W': 'right'}
valign = {'N': 'bottom', 'S': 'top', 'E': 'center', 'W': 'center'}
pivot = {'N': 'mid', 'S': 'mid', 'E': 'tip', 'W': 'tail'}

def __init__(self, Q, X, Y, U, label, **kw):
martist.Artist.__init__(self)
Expand All @@ -236,13 +236,21 @@ def __init__(self, Q, X, Y, U, label, **kw):
self._labelsep_inches = kw.pop('labelsep', 0.1)
self.labelsep = (self._labelsep_inches * Q.ax.figure.dpi)

# try to prevent closure over the real self
weak_self = weakref.ref(self)
Copy link
Member

Choose a reason for hiding this comment

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

This use of weak ref looks fine (and I can't think of an alternative way)


def on_dpi_change(fig):
self.labelsep = (self._labelsep_inches * fig.dpi)
self._initialized = False # simple brute force update
# works because _init is called
# at the start of draw.
_s = weak_self()
Copy link
Member

Choose a reason for hiding this comment

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

Not a fan of the variable name - how about self_weakref?

if _s is not None:
_s.labelsep = (_s._labelsep_inches * fig.dpi)
_s._initialized = False # simple brute force update
# works because _init is called
# at the start of draw.

self._cid = Q.ax.figure.callbacks.connect('dpi_changed',
on_dpi_change)

Q.ax.figure.callbacks.connect('dpi_changed', on_dpi_change)
self._cb_ref = weakref.ref(Q.ax.figure.callbacks)
Copy link
Member

Choose a reason for hiding this comment

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

Since we already have a non-weak ref to Q, could we not just remove this and below (in remove), do Q.ax.figure.callbacks instead? (Haven't tried, just seems like the simpler solution that also doesn't create any extra strong references.)

Copy link
Member Author

Choose a reason for hiding this comment

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

had blinders on and missed that we had that ref to Q


self.labelpos = kw.pop('labelpos', 'N')
self.labelcolor = kw.pop('labelcolor', None)
Expand All @@ -255,11 +263,24 @@ def on_dpi_change(fig):
horizontalalignment=self.halign[self.labelpos],
verticalalignment=self.valign[self.labelpos],
fontproperties=font_manager.FontProperties(**_fp))

if self.labelcolor is not None:
self.text.set_color(self.labelcolor)
self._initialized = False
self.zorder = Q.zorder + 0.1

def remove(self):
"""
Overload the remove method
"""
_cbs = self._cb_ref()
if _cbs is not None:
# disconnect the call back
_cbs.disconnect(self._cid)
self._cid = None
# pass the remove call up the stack
martist.Artist.remove(self)

__init__.__doc__ = _quiverkey_doc

def _init(self):
Expand Down