esheldon / pymangle

Simple code to read mangle masks, calculate ids and weights, and generate random points
14 stars 15 forks source link

contains() to return bool array #3

Closed pmelchior closed 8 years ago

pmelchior commented 8 years ago

Currently contains(ra, dec) returns an integer array with 0 and 1. That's not economical and also leads to a problems when using boolean indexing in numpy:

N = ra.size
inside = m.contains(ra, dec)
x = ra[inside]

will yield x with the size N, made from repeated entries of ra[0] and ra[1], because inside is an integer array. That's counter-intuitive.

I made a simple addition to create boolean arrays and use such an array in contains.

esheldon commented 8 years ago

I'm worried too many people now expect an integer array to be returned. This could break peoples code in unexpected ways.

pmelchior commented 8 years ago

I'm not sure how this could lead to problems. Currently, one can only test for 1 at the one of those, and that still works with a boolean array. In particular, if someone were to go through those and perform numpy slices, they'd had to do this:

inside = m.contains(ra, dec)
mask = (inside == 1)
x = ra[mask]

To be clear: The integer indexing method does not work now, and the boolean indexing requires the second step above.

esheldon commented 8 years ago

I didn't realize that you could test that boolean arrays equal 1

w,=numpy.where(boolarr == 1)
# equivalent to
w,=numpy.where(boolarr == True)

# and

w,=numpy.where(boolarr == 0)
# equivalent to
w,=numpy.where(boolarr == False)

OK, I think we can probably merge this then. Have you tested it in the wild?

pmelchior commented 8 years ago

If by "the wild" you mean my machine (a rather tame MacBook Pro), then yes. And yes, python/numpy is clever enough to figure the 1/True thing out.

esheldon commented 8 years ago

I mean has it been run in a pipeline by some complex tool like balrog?

esheldon commented 8 years ago

Also, if aurelian could test that would be helpful, not sure of his github tag

esheldon commented 8 years ago

The DES mangle pipeline depends on this code is why I would like his input

pmelchior commented 8 years ago

Absolutely, I don't mean to break anything.