__contains__ scope?

[joleroi]

At the moment the Region base classes have an abstract method: __contains__

https://github.com/astropy/regions/blob/master/regions/core/core.py#L195

The docstring suggests that this is supposed to work for a list of SkyCoords

https://github.com/astropy/regions/blob/master/regions/core/core.py#L197

I don't think this works/is a good idea

In [35]: c1
Out[35]:
CircleSkyRegion
Center:<SkyCoord (Galactic): (l, b) in deg
    (0.0, 0.0)>
Radius:1.0 deg

In [36]: coords
Out[36]:
<SkyCoord (Galactic): (l, b) in deg
    [(1.2, 1.2), (0.5, 0.5), (0.7, 0.7)]>

In [37]: c1.__contains__(coords)
Out[37]: array([False,  True,  True], dtype=bool)

In [38]: coords in c1
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
/tmp/test.py in <module>()
----> 1 coords in c1

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

I looks like the in operator checks for a (non-array) boolean output of __contains__. This makes sense if you think about statements like

if coord in regions:
    do something

I would suggest changing the API to regions.contains(coords) (without the underscores).

Do you agree @keflavich @astrofrog @bsipocz ?

[astrofrog]

Sounds good, but we can leave __contains__ for scalar cases?

[joleroi]

cc @cdeil

[cdeil]

For the record:

While writing the docs for region containment checks
https://github.com/astropy/regions/pull/50/files#diff-9bc8bf6e8d9db020cefce1fbc0426795R189
I had this same question, whether points in region should / could work for array-valued points.

There's some information here what in really does:
http://stackoverflow.com/questions/12244074/python-source-code-for-built-in-in-operator
http://stackoverflow.com/questions/37613288/in-statement-for-lists-tuples-of-floats

I used this example to play around:

import numpy as np

class PixCoord:
    def __init__(self, x, y):
        self.x = x
        self.y = y

    def isscalar(self):
        return np.isscalar(self.x)

class PixelRegion:
    def __init__(self, center, radius):
        self.center = center
        self.radius = radius

    def __contains__(self, coord):
        dx = self.center.x - np.array(coord.x)
        dy = self.center.y - np.array(coord.y)
        d = np.sqrt(dx * dx + dy * dy)
        mask = d < self.radius
        print(mask)
        return mask


c = PixCoord(x=1, y=2)
c2 = PixCoord(x=[1, 2], y=[2, 2])
r = PixelRegion(center=PixCoord(0, 0), radius=3)
print(c in r)
print(c2 in r)

The c2 in r statement results in

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

without a further traceback what happened.
I think effectively Python is doing something like calling bool(retval) on the return value of __contains__, doing this manually return bool(mask) gives the same error.

So my conclusion is the same one as @joleroi's -- making in work for arrays of points is not possible (or if it is, I presume it's not a great idea).

I'm not sure it's a good idea if having __contains__ in addition to contains with a subset of the functionality is useful. My suggestion would be to remove it and have one and only one way to do it: region.contains(coords). But I don't care strongly, so I'm not re-opening this issue for discussion.