8000 Don't warn in Collections.contains if picker is not numlike. by anntzer · Pull Request #6491 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Don't warn in Collections.contains if picker is not numlike. #6491

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

Conversation

anntzer
Copy link
Contributor
@anntzer anntzer commented May 28, 2016

Otherwise, a warning is raised in the simple example:

from pylab import *

def pick_test(artist, mouseevent):
    return coll.contains(mouseevent)

coll = plt.scatter([0, 1], [0, 1], picker=pick_test)
plt.show()

(Click anywhere in the figure to trigger the warning.)

The use of is_numlike matches the implementation of Line2D.contains.

Initially noted while using mpldatacursor.

@anntzer
Copy link
Contributor Author
anntzer commented Jul 7, 2016

Bump, this should be fairly non-controversial? I could add an explicit test that _picker is a callable if not a numlike (but this should probably happen in the setter instead).

@WeatherGod
Copy link
Member

First, the PR needs to be rebased. Second, the comment for the original warning gives me pause. It is saying that it shouldn't happen unless it is being called in a non-normal manner. So... is the comment still correct and something else is wrong, or is the comment outdated?

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jul 7, 2016
@tacaswell
Copy link
Member

I agree the behavior is the same, that comment makes me a little nervous though. I suspect that it is technical debt from an old check that got over run by new functionality.

I looked at this a while ago and abandoned my changes, but I no longer remember why (could have been I found a problem or could have been I switched to something else and just never came back 😕 ) .

Needs a rebase, I am +0.5 on merging.

@tacaswell
Copy link
Member

It as also distressing how often @WeatherGod and I leave very similar comments with in minutes of each other.

@anntzer anntzer force-pushed the dont-warn-in-collections-picker branch from bacc4e0 to 7465773 Compare July 7, 2016 15:29
@anntzer
Copy link
Contributor Author
anntzer commented Jul 7, 2016

I guess the check and the comment must date back from before it was allowed to pass an arbitrary callable as the picker kwarg (which is now explicitly allowed by the docs)?

Otherwise, a warning is raised in the simple example:

    from pylab import *

    def pick_test(artist, mouseevent):
        return coll.contains(mouseevent)

    coll = plt.scatter([0, 1], [0, 1], picker=pick_test)
    plt.show()

(Click anywhere in the figure to trigger the warning.)

The use of `is_numlike` matches the implementation of `Line2D.contains`.

Initially noted while using mpldatacursor.
@anntzer anntzer force-pushed the dont-warn-in-collections-picker branch from 7465773 to c5ec5b3 Compare August 26, 2016 03:56
@anntzer
Copy link
Contributor Author
anntzer commented Aug 26, 2016

Rebased; bumping.

@WeatherGod
Copy link
Member

Test failure is pytest-only and is related to stixsans font.

@tacaswell tacaswell merged commit d158587 into matplotlib:master Aug 26, 2016
@anntzer anntzer deleted the dont-warn-in-collections-picker branch August 26, 2016 17:50
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.

4 participants
0