Skip to content

Allow Artists to show pixel data in cursor display #3989

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.

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 26 commits into from
May 24, 2015

Conversation

blink1073
Copy link
Member

Also adds implementation for AxesImage. Closes #3984.
Inspired by mpldatacursor.

screenshot from 2015-02-21 12 13 27

screenshot from 2015-02-21 12 14 20

@tacaswell tacaswell added this to the v1.5.x milestone Jan 9, 2015
@blink1073 blink1073 changed the title Allow Artists to Diplay Zdata in Cursor Allow Artists to Display Zdata in Cursor Jan 10, 2015
@@ -2791,6 +2791,12 @@ def mouse_move(self, event):
except (ValueError, OverflowError):
pass
else:
ax = event.inaxes
artists = ax.artists + ax.images + ax.lines
Copy link
Member

Choose a reason for hiding this comment

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

Should also look at ax.collections (that is where things like scatter live) and maybe ax.patches

Copy link
Member

Choose a reason for hiding this comment

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

and the order should be ax.collections + ax.patches + ax.lines + ax.artists + ax.images to match the order used for sorting out the rendering order in Axes.draw (around https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/axes/_base.py#L1993 images are special cased around line 2030).

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Right, but the ordering is a stable sort so ties are broken by which was in the list first. This is the source of at least one un-fixable (with out major overhaul of the internal data structure, see #3944) bug ( #409 ).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok then. So how to handle composite images? It does not seem feasible to recreate the composite every time.

Copy link
Member Author

Choose a reason for hiding this comment

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

The AxesImage could composite the specific pixel on demand.

Copy link
Member

Choose a reason for hiding this comment

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

I think to first order, just ignore the compositing and use the top image.

The in-place compositing is only for some of the vector backends which can't do it them selves (ex pdf) to try and save space (so you don't have multiple overlapping images) but it leads to other bugs as all images get smashed to the same zorder relative to the other lines etc. In Agg (which backs all but 3 (gtkcairo, macsox and windows) of the interactive backends) the whole if _do_composite block is skipped and the compsiting is taken care of at the c++ layer.

@blink1073
Copy link
Member Author

How's that? I noticed onRemove was doing something similar and found Artist.hitlist(), and standardized between the two.

@blink1073
Copy link
Member Author

However, it looks like Axes.get_children needs to return the same order as Axes.draw. New PR for that?

@tacaswell
Copy link
Member

onRemove looks like another candidate (along with onHilite) for removal from FigureCanvasBase (now that you have simplified it)....

Probably worth having a top-level module for useful event handlers, which I think we greatly reduce the threshold to get people using the event system. @WeatherGod @joferkington thoughts?

Yes to separate PR for re-ordering Axes.get_children as that is a (subtle) api change.

@blink1073
Copy link
Member Author

I think we do need to handle compositing. For instance, if I run this example, I get the Zdata for the overlay layer. How about just checking for alpha=0 and then using the underlying in that case?

@blink1073
Copy link
Member Author

Well, the more I think about it the more I agree that the top image should get priority. The canny value really is the point of the example.

@blink1073
Copy link
Member Author

Added Artist.format_zdata to centralize formatting.

@blink1073
Copy link
Member Author

Do I still "need revision"?

@tacaswell
Copy link
Member

Looks good to me. I am a tad wary of the format_z method which does not use self, but that seems like a reasonable place to put it.

I would like @WeatherGod to weigh in.

@blink1073
Copy link
Member Author

What do you mean "does not use self"?

@blink1073
Copy link
Member Author

Meaning it should be a static method?

@blink1073
Copy link
Member Author

Either way I think it belongs in the class so it can be overridden.

@tacaswell
Copy link
Member

Yes, it is an instance method which does not refer to it's instance and I can't think of a use case where you would want to (but I may just have poor imagination). I thought about suggesting to move it to cbook, but it is much too specific for that.

@blink1073
Copy link
Member Author

I'd worry that adding a @staticmethod would give people the impression they could not subclass it as an instance method (I checked, and you can).

@tacaswell
Copy link
Member

@blink1073
Copy link
Member Author

Done.

@blink1073 blink1073 changed the title Allow Artists to Display Zdata in Cursor Allow Artists to display zdata in cursor Jan 26, 2015
@tacaswell tacaswell modified the milestones: next point release, proposed next point release Feb 17, 2015
@tacaswell
Copy link
Member

@matplotlib/developers Can this get eyes from someone else but me?

This addresses a long standing and frequent feature request in a pleasingly general way.

"""
Get the zdata for a given event, as a string message
"""
return ''
Copy link
Member

Choose a reason for hiding this comment

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

Why do you want to use a string for this? Shouldn't "data" always be numeric?

Copy link
Member Author

Choose a reason for hiding this comment

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

This allows the Artist to control the display of numbers, similar to the existing ax.format_coord.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @efiring on this one. I can think of a lot of use cases (e.g. interactive sampling) where it would be very convenient to access a "raw" value. Returning a numerical value would make this method useful for a lot more than just display.

You already have a separate method for the formatting, so it's only slightly more verbose to flip the function calls on their head and make the displayed string using artist.format_zdata(artist.get_zdata()). In my opinion, this would be a better approach overall.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha! I originally meant to do that...

@tacaswell
Copy link
Member

@joferkington again. Sorry to pester, but I see you are doing OSS stuff today and would like your feed back on this 😄

@joferkington
Copy link
Contributor

@tacaswell - Absolutely! Sorry I've been so out-of-touch with things lately, as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Scalar Image Cursor Display?
7 participants