NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

tacaswell merged 26 commits intomatplotlib:masterfromblink1073:add-cursor-zdata

May 24, 2015

Merged

Allow Artists to show pixel data in cursor display#3989

tacaswell merged 26 commits intomatplotlib:masterfromblink1073:add-cursor-zdata

May 24, 2015

Conversation

Copy link

Member

blink1073 commentedJan 9, 2015

Also adds implementation forAxesImage.Closes#3984.
Inspired bympldatacursor.

tacaswell added the status: needs review label

Jan 9, 2015

tacaswell added this to thev1.5.x milestone

Jan 9, 2015

blink1073 changed the title~~Allow Artists to Diplay Zdata in Cursor~~Allow Artists to Display Zdata in Cursor

Jan 10, 2015

tacaswell reviewed

Jan 19, 2015

View reviewed changes

lib/matplotlib/backend_bases.py

		@@ -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 atax.collections (that is where things like scatter live) and maybeax.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 beax.collections + ax.patches + ax.lines + ax.artists + ax.images to match the order used for sorting out the rendering order inAxes.draw (aroundhttps://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/axes/_base.py#L1993 images are special cased around line 2030).

Copy link

MemberAuthor

blink1073Jan 19, 2015

Choose a reason for hiding this comment

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

They then get ordered by zorder anyway:https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/axes/_base.py#L2036

Copy link

Member

tacaswellJan 19, 2015

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

MemberAuthor

blink1073Jan 19, 2015

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

MemberAuthor

blink1073Jan 19, 2015

Choose a reason for hiding this comment

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

TheAxesImage could composite the specific pixel on demand.

Copy link

Member

tacaswellJan 19, 2015

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 wholeif _do_composite block is skipped and the compsiting is taken care of at the c++ layer.

tacaswell added status: needs revision and removed status: needs review labels

Jan 19, 2015

Copy link

MemberAuthor

blink1073 commentedJan 19, 2015

How's that? I noticedonRemove was doing something similar and foundArtist.hitlist(), and standardized between the two.

Copy link

MemberAuthor

blink1073 commentedJan 19, 2015

However, it looks likeAxes.get_children needs to return the same order asAxes.draw. New PR for that?

Copy link

Member

tacaswell commentedJan 19, 2015

onRemove looks like another candidate (along withonHilite) for removal fromFigureCanvasBase (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-orderingAxes.get_children as that is a (subtle) api change.

Copy link

MemberAuthor

blink1073 commentedJan 19, 2015

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

Copy link

MemberAuthor

blink1073 commentedJan 19, 2015

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.

Copy link

MemberAuthor

blink1073 commentedJan 19, 2015

AddedArtist.format_zdata to centralize formatting.

Copy link

MemberAuthor

blink1073 commentedJan 20, 2015

Do I still "need revision"?

tacaswell added status: needs review and removed status: needs revision labels

Jan 20, 2015

Copy link

Member

tacaswell commentedJan 20, 2015

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

I would like@WeatherGod to weigh in.

Copy link

MemberAuthor

blink1073 commentedJan 20, 2015

What do you mean "does not useself"?

Copy link

MemberAuthor

blink1073 commentedJan 20, 2015

Meaning it should be a static method?

Copy link

MemberAuthor

blink1073 commentedJan 20, 2015

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

Copy link

Member

tacaswell commentedJan 20, 2015

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 tocbook, but it is much too specific for that.

Copy link

MemberAuthor

blink1073 commentedJan 20, 2015

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).

Copy link

Member

tacaswell commentedJan 20, 2015

This also needs an entry inhttps://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new

Copy link

MemberAuthor

blink1073 commentedJan 20, 2015

Done.

blink1073 changed the title~~Allow Artists to Display Zdata in Cursor~~Allow Artists to display zdata in cursor

Jan 26, 2015

tacaswell modified the milestones:next point release,proposed next point release

Feb 17, 2015

Copy link

Member

tacaswell commentedFeb 17, 2015

@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.

efiring reviewed

Feb 17, 2015

View reviewed changes

lib/matplotlib/artist.py

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

Copy link

Member

efiringFeb 17, 2015

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

MemberAuthor

blink1073Feb 17, 2015

Choose a reason for hiding this comment

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

This allows theArtist to control the display of numbers, similar to the existingax.format_coord.

Copy link

Contributor

joferkingtonFeb 20, 2015

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 usingartist.format_zdata(artist.get_zdata()). In my opinion, this would be a better approach overall.

Copy link

MemberAuthor

blink1073Feb 21, 2015

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...

Copy link

Member

tacaswell commentedFeb 20, 2015

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

Copy link

Contributor

joferkington commentedFeb 20, 2015

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

blink1073 added20 commits

March 17, 2015 15:56

Fix the artist sorting behaviour

8ea9504

Fix syntax error

02456b8

Specify hitlist on axes instead of figure

641304c

Put get_zdata on the correct class

3f20d3a

Ignore the axes patch

5850150

Create new function to format zdata

88b56be

Clean up string formatting

aece695

Add a whats new blurb.

07745c8

Add a test for image cursor data

7626394

Rename to get_pixel_data and return raw data

7b0b6b1

Update the whats new

f5ff73f

Fix name of method

ea565c6

Fix default pixel data and handle no pixel data

1429206

Update test for new method name

e62b0e3

Simplify transform

34b4df4

Update display

8f266a1

Simplify format_pixel_data

70983ff

Update name to cursor_data

cd0e52b

Move artist sort to within if block

8ce3a6f

Fix method name in cursor_test

bac8dff

blink1073 force-pushed theadd-cursor-zdata branch fromdf4ee01 tobac8dffCompare

March 17, 2015 20:56

This was referencedMar 27, 2015

Twin axis message coordinates#4284

Closed

corrplot does not show cell coordinates with nbagg backendmwaskom/seaborn#495

Closed

michaelaye mentioned this pull request

Apr 1, 2015

Heatmap mouseovermwaskom/seaborn#496

Closed

tacaswell added a commit that referenced this pull request

May 24, 2015

Merge pull request#3989from blink1073/add-cursor-zdata

c473c1b

ENH: Allow Artists to show pixel data in cursor display

tacaswell merged commitc473c1b intomatplotlib:master

May 24, 2015

tacaswell removed the status: needs review label

May 24, 2015

tacaswell mentioned this pull request

Jul 16, 2015

Display z value in matshow in addition of x, y.#3146

Closed

tacaswell mentioned this pull request

Jul 26, 2015

Regression Qt5 Backend#4767

Closed

megies mentioned this pull request

Jan 31, 2018

fix detecting which artist(s) the mouse is over#10356

Merged

6 tasks

Labels

None yet

7 participants

Movatterモバイル変換

Uh oh!

Allow Artists to show pixel data in cursor display#3989

Allow Artists to show pixel data in cursor display#3989

Uh oh!

Conversation

blink1073 commentedJan 9, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blink1073 commentedJan 19, 2015

Uh oh!

blink1073 commentedJan 19, 2015

Uh oh!

tacaswell commentedJan 19, 2015

Uh oh!

blink1073 commentedJan 19, 2015

Uh oh!

blink1073 commentedJan 19, 2015

Uh oh!

blink1073 commentedJan 19, 2015

Uh oh!

blink1073 commentedJan 20, 2015

Uh oh!

tacaswell commentedJan 20, 2015

Uh oh!

blink1073 commentedJan 20, 2015

Uh oh!

blink1073 commentedJan 20, 2015

Uh oh!

blink1073 commentedJan 20, 2015

Uh oh!

tacaswell commentedJan 20, 2015

Uh oh!

blink1073 commentedJan 20, 2015

Uh oh!

tacaswell commentedJan 20, 2015

Uh oh!

blink1073 commentedJan 20, 2015

Uh oh!

tacaswell commentedFeb 17, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tacaswell commentedFeb 20, 2015

Uh oh!

joferkington commentedFeb 20, 2015

Uh oh!

Uh oh!