PRF: only check some artists on mousemove #4878

tacaswell · 2015-08-07T04:11:53Z

Instead of walking the full hitlist of the Axes, only check artists
that ask to be part of the mouseover events.

This is still missing logic to remove the artist from the mouseover_set
when removing the artist from the Axes.

This is an alternative to #4847

Instead of walking the full hitlist of the Axes, only check artists that ask to be part of the mouseover events. This is still missing logic to remove the artist from the mouseover_set when removing the artist from the Axes.

blink1073 · 2015-08-07T13:05:56Z

What would be wrong with checking a.mouseover directly and avoiding mouseover_set?

tacaswell · 2015-08-07T13:10:25Z

I was trying to avoid walking the artist tree at all and avoid changing hit
list which i assume is used else where.

On the other hand, self.get_children would only do one layer, be only a bit
slower (I am guessing which I know is always a bad idea) and be a lot
simpler.

On Fri, Aug 7, 2015, 9:05 AM Steven Silvester notifications@github.com
wrote:

What would be wrong with checking a.mouseover directly and avoiding
mouseover_set?

—
Reply to this email directly or view it on GitHub
#4878 (comment)
.

blink1073 · 2015-08-07T13:14:11Z

You're right, it makes more sense the way it is now.

fariza · 2015-08-07T14:37:17Z

I prefer this approach to using the hitlist, it's cleaner and easier to follow what's being reported.

@tacaswell I think this would be a good place to also address #4284 making, a similar property in axes, to report or not.

tacaswell · 2015-08-10T05:27:23Z

@fariza Possibly, but by the time we get to this code, the decision about what Axes to hand the event to to process has already been made, although it is probably simple to look up if there are any twin axes here.

I am inclined to punt on that to get the mouse-over code shippable ASAP.

WeatherGod · 2015-08-12T18:54:30Z

Initially reported over in #4738 , I encountered a very troubling bug with NavigationToolbar2.mouse_over. When I display a very large image (haven't tried smaller images), and I pass the mouse over the figure window, memory usage increases by 100s of MBs every few seconds, and it doesn't seem to stop increasing. I eventually have to kill -9 the process when it gets up to around 5GB. This behavior is present even with this PR.

tacaswell · 2015-08-14T01:26:31Z

Also close #4918

This is useful when removing artists from an `Axes`

- disconnect the stale_callback (this assumes that matplotlib#4738 goes in) - remove the artist from the Axes.mousover_set - marks the parent axes/figure as stale - resets the artist's axes/figure to None

tacaswell · 2015-08-14T02:01:06Z

@matplotlib/developers I think this is ready to go.

tacaswell · 2015-08-14T12:08:50Z

Forgot this last thing from been when i was working on this/writing up the
current state last night.

On Wed, Aug 12, 2015, 2:54 PM Benjamin Root notifications@github.com
wrote:

Initially reported over in #4738
#4738 , I encountered a
very troubling bug with NavigationToolbar2.mouse_over. When I display a
very large image (haven't tried smaller images), and I pass the mouse over
the figure window, memory usage increases by 100s of MBs every few seconds,
and it doesn't seem to stop increasing. I eventually have to kill -9 the
process when it gets up to around 5GB. This behavior is present even with
this PR.

—
Reply to this email directly or view it on GitHub
#4878 (comment)
.

tacaswell · 2015-08-14T12:09:51Z

Been should be Ben.

On Fri, Aug 14, 2015, 8:08 AM Thomas Caswell tcaswell@gmail.com wrote:

Forgot this last thing from been when i was working on this/writing up the
current state last night.

On Wed, Aug 12, 2015, 2:54 PM Benjamin Root notifications@github.com
wrote:

Initially reported over in #4738
#4738 , I encountered a
very troubling bug with NavigationToolbar2.mouse_over. When I display a
very large image (haven't tried smaller images), and I pass the mouse over
the figure window, memory usage increases by 100s of MBs every few seconds,
and it doesn't seem to stop increasing. I eventually have to kill -9 the
process when it gets up to around 5GB. This behavior is present even with
this PR.

—
Reply to this email directly or view it on GitHub
#4878 (comment)
.

WeatherGod · 2015-08-14T13:54:48Z

I did a little bit more investigation on this. The data I was displaying
were masked arrays from netCDF4 objects. Meanwhile, if I simply create
large masked arrays with random data, I cannot reproduce the problem. So,
perhaps there is something about how we are interacting with views of data
that (I believe) is memmapped?

On Fri, Aug 14, 2015 at 8:09 AM, Thomas A Caswell notifications@github.com
wrote:

Been should be Ben.

On Fri, Aug 14, 2015, 8:08 AM Thomas Caswell tcaswell@gmail.com wrote:

Forgot this last thing from been when i was working on this/writing up
the
current state last night.

On Wed, Aug 12, 2015, 2:54 PM Benjamin Root notifications@github.com
wrote:

Initially reported over in #4738
#4738 , I encountered a
very troubling bug with NavigationToolbar2.mouse_over. When I display a
very large image (haven't tried smaller images), and I pass the mouse
over
the figure window, memory usage increases by 100s of MBs every few
seconds,
and it doesn't seem to stop increasing. I eventually have to kill -9 the
process when it gets up to around 5GB. This behavior is present even
with
this PR.

—
Reply to this email directly or view it on GitHub
<
https://github.com/matplotlib/matplotlib/pull/4878#issuecomment-130411057>
.

—
Reply to this email directly or view it on GitHub
#4878 (comment)
.

jenshnielsen · 2015-08-14T14:19:03Z

The test error on python 2.6 looks real

ERROR: Failure: AttributeError ('module' object has no attribute 'test_artist')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/matplotlib/matplotlib/venv/lib/python2.6/site-packages/nose/loader.py", line 407, in loadTestsFromName
    module = resolve_name(addr.module)
  File "/home/travis/build/matplotlib/matplotlib/venv/lib/python2.6/site-packages/nose/util.py", line 322, in resolve_name
    obj = getattr(obj, part)
AttributeError: 'module' object has no attribute 'test_artist'

tacaswell · 2015-08-14T14:21:08Z

The failure is due to import errors in test_artist in 2.6, hurray.

I really want to just drop 2.6, but I will get this sorted out.

jenshnielsen · 2015-08-14T14:24:48Z

Im 👍 on dropping python 2.6 but I guess should keep it round for 1.5 and 2.0. Skipping the tests on Python 2.6 is IMHO an acceptable solution if the import error is in the test.

OceanWolf · 2015-08-14T14:25:33Z

Drop 2.6??? Could we do that for 2.x? OrderedDicts here we come! 😄.

Apparently `assert_is`, `assert_in`, and `assert_not_in` are not in the 2.6 version of nose, but are in all others.

tacaswell · 2015-08-14T14:29:37Z

The problem was that some of the fancier assert statements are not in 2.6, but are every where else (despite nose having the same version). I just switched to using assert_true which is easier than setting up conditional tests and local imports.

PRF: only check some artists on mousemove

pelson · 2015-08-27T05:59:28Z

lib/matplotlib/artist.py

@@ -961,6 +980,21 @@ def format_cursor_data(self, data):
            data = [data]
        return ', '.join('{:0.3g}'.format(item) for item in data)

+    @property
+    def mouseover(self):


Just wondering, have you ever measured the performance of descriptor access vs attribute lookup. You might be quite shocked 😱

I have not, but I made this a property for the setter functionality.

tacaswell · 2015-08-27T12:46:15Z

@pelson I agree this isn't great, but the hitlist solution was taking longer to run than the time between mouse events and the automatic mouse-over functionality is one people have been asking for for years.

Another option I see is to periodically walk the children and cache which of them have a message to add to the mouse over string, but that has a higher chance of going out of sync than this method does.

I have no problem renaming mouseover_set.

pelson · 2015-09-03T08:35:43Z

lib/matplotlib/artist.py

@@ -147,6 +147,23 @@ def remove(self):
        # callback has one parameter, which is the child to be removed.
        if self._remove_method is not None:
            self._remove_method(self)


Surely the logic for removing from the axes should live in the remove method itself. Is there a reason we can't do that?

PRF: only check some artists on mousemove

0def4c5

Instead of walking the full hitlist of the Axes, only check artists that ask to be part of the mouseover events. This is still missing logic to remove the artist from the mouseover_set when removing the artist from the Axes.

tacaswell added this to the next point release milestone Aug 7, 2015

tacaswell added the status: needs review label Aug 7, 2015

This was referenced Aug 10, 2015

ENH: make mouse over behavior configurable #4847

Closed

outstanding issues from jpl #4897

Closed

tacaswell mentioned this pull request Aug 14, 2015

Notebook backend zoom rect VEEERY slow in 1.5dev #4918

Closed

tacaswell added 2 commits August 13, 2015 21:34

API: allow setting axes back to None

1133214

This is useful when removing artists from an `Axes`

ENH: pull apart Artist state when removing

01c4b90

- disconnect the stale_callback (this assumes that matplotlib#4738 goes in) - remove the artist from the Axes.mousover_set - marks the parent axes/figure as stale - resets the artist's axes/figure to None

TST: added test of remove functionality

090b6d6

MNT: not all assert methods available in 2.6

ded655a

Apparently `assert_is`, `assert_in`, and `assert_not_in` are not in the 2.6 version of nose, but are in all others.

jenshnielsen added a commit that referenced this pull request Aug 16, 2015

Merge pull request #4878 from tacaswell/prf_mouse_move_event

be93150

PRF: only check some artists on mousemove

jenshnielsen merged commit be93150 into matplotlib:master Aug 16, 2015

tacaswell removed the status: needs review label Aug 16, 2015

jenshnielsen mentioned this pull request Aug 16, 2015

Memory leak in NavigationToolbar2.mouse_over #4936

Closed

tacaswell deleted the prf_mouse_move_event branch August 17, 2015 00:02

pelson reviewed Aug 27, 2015
View reviewed changes

pelson reviewed Sep 3, 2015
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PRF: only check some artists on mousemove #4878

PRF: only check some artists on mousemove #4878

tacaswell commented Aug 7, 2015

blink1073 commented Aug 7, 2015

tacaswell commented Aug 7, 2015

blink1073 commented Aug 7, 2015

fariza commented Aug 7, 2015

tacaswell commented Aug 10, 2015

WeatherGod commented Aug 12, 2015

tacaswell commented Aug 14, 2015

tacaswell commented Aug 14, 2015

tacaswell commented Aug 14, 2015

tacaswell commented Aug 14, 2015

WeatherGod commented Aug 14, 2015

jenshnielsen commented Aug 14, 2015

tacaswell commented Aug 14, 2015

jenshnielsen commented Aug 14, 2015

OceanWolf commented Aug 14, 2015

tacaswell commented Aug 14, 2015

pelson Aug 27, 2015

tacaswell Aug 27, 2015

tacaswell commented Aug 27, 2015

pelson Sep 3, 2015

PRF: only check some artists on mousemove #4878

PRF: only check some artists on mousemove #4878

Conversation

tacaswell commented Aug 7, 2015

blink1073 commented Aug 7, 2015

tacaswell commented Aug 7, 2015

blink1073 commented Aug 7, 2015

fariza commented Aug 7, 2015

tacaswell commented Aug 10, 2015

WeatherGod commented Aug 12, 2015

tacaswell commented Aug 14, 2015

tacaswell commented Aug 14, 2015

tacaswell commented Aug 14, 2015

tacaswell commented Aug 14, 2015

WeatherGod commented Aug 14, 2015

jenshnielsen commented Aug 14, 2015

tacaswell commented Aug 14, 2015

jenshnielsen commented Aug 14, 2015

OceanWolf commented Aug 14, 2015

tacaswell commented Aug 14, 2015

pelson Aug 27, 2015

Choose a reason for hiding this comment

tacaswell Aug 27, 2015

Choose a reason for hiding this comment

tacaswell commented Aug 27, 2015

pelson Sep 3, 2015

Choose a reason for hiding this comment