| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
This fix seems right. Can you rebase against a recent master - your test failure should be fixed in recent changes to master (I think). I think you are correct that the return from contains() could have been better, but I think you are alos correct that changing it would be a bit of a project. |
Sorry, something went wrong.
Yeah. Changing it would be simple, but it will definitely hurt people that use .contains() in third party code. So, yeah, not an option, I figured.
I thought bug fixes used to be against v2.1.x (hmm.. I think they used to be called maintenance_...?).. did I miss a change in the workflow? Are bug fixes now also made against master and then backported? If yes, then I can rebase, no problem. |
Sorry, something went wrong.
|
maintenance_... is ObsPy, not Matplotlib. There are no more 2.1.z releases planned, so it's best to target master. In any case, it'll be backported if that changes. |
Sorry, something went wrong.
|
Alright, rebased on master and switched base branch.. |
Sorry, something went wrong.
|
@megies. Sorry this fell off the radar. It’s still not passing tests though I’m not sure if that is the prs fault. Can you ping if it’s done? |
Sorry, something went wrong.
There was a problem hiding this comment.
With a rebase...
Sorry, something went wrong.
since Artist.contains() returns a tuple it is not usable like "if my_artist.contains(): do something"
|
Rebase done. |
Sorry, something went wrong.
There was a problem hiding this comment.
anyone can merge post-CI
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
There is a bug in how moving the mouse over the plot detects what artist should be used to collect additional info about the cursor position from (in my case several Images in the axes). For a plot with multiple images this means that only one image is properly reporting z coordinates and all other images won't be able to show z coordinate info.
Example:
Only moving through one of the images shows correct z coordinates in the interactive window info bar at the bottom.
The reason for this bug is an oversight in #3989:
Since Artist.contains() returns a tuple it is not usable like this:
So, what is happening is that all artists are included in the list of artists that contain the event and then the wrong artist is queried for data on the location of the event.
Since the name of the method "contains" might easily in the future be prone to such bugs again, I think a better solution would be to change the return from a 2-tuple (which will always evaluate to True) to only False (not contained) or a dictionary (contained) that in minimal fashion is set up like {'artist': self} so that it evaluates to True. But obviously that would drastically change a public API, so I'm assuming this won't be an option, likely..
Sorry, I'm not sure how to best test this, I think to add a test would mean to first refactor NavigationToolbar2.mouse_move() so that getting the cursor info from all artists is in a separate helper method..
PR Checklist