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

Merged
merged 1 commit into from Aug 26, 2016

Conversation

Projects
None yet
4 participants
Contributor

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.

mdboom added the needs_review label May 28, 2016

Contributor

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

Member

WeatherGod commented Jul 7, 2016

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 added this to the 2.1 (next point release) milestone Jul 7, 2016

Owner

tacaswell commented Jul 7, 2016

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.

Owner

tacaswell commented Jul 7, 2016

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

Contributor

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

@anntzer anntzer Don't warn in Collections.contains if picker is not numlike.
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.
c5ec5b3
Contributor

anntzer commented Aug 26, 2016

Rebased; bumping.

Member

WeatherGod commented Aug 26, 2016

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

@tacaswell tacaswell merged commit d158587 into matplotlib:master Aug 26, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.0002%) to 70.201%
Details

tacaswell removed the needs_review label Aug 26, 2016

anntzer deleted the anntzer:dont-warn-in-collections-picker branch Aug 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment