add unit test demonstrating failure of Brick.brick_radec() on scalar inputs

[dstndstn]

Fails with:

ERROR: test_brick_radec_scalar (desiutil.test.test_brick.TestBrick)
Test scalar to brick RA,Dec conversion.

Traceback (most recent call last):
File "/Users/dstn/desiutil/py/desiutil/test/test_brick.py", line 258, in test_brick_radec_scalar
ra,dec = b.brick_radec(0., 0.)
File "/Users/dstn/desiutil/py/desiutil/brick.py", line 284, in brick_radec
xra = self._center_ra[irow][icol]
TypeError: only integer scalar arrays can be converted to a scalar index

---

irow and icol are numpy arrays of integers with one element each.

[weaverba137]

        if np.isscalar(ra):
            xra = self._center_ra[irow][icol]
            xdec = self._center_dec[irow]

needs to be changed to

        if np.isscalar(ra):
            xra = self._center_ra[irow[0]][icol]
            xdec = self._center_dec[irow]

[weaverba137]

Are you intending to include fixes to the large-size bricks in this PR?

[weaverba137]

I don't know if you intend to include the fixes for large bricks in this PR, but at minimum you need to update the changes.rst file as well.

[weaverba137]

Please change the __doc__ string. You're testing an array, not a scalar.

[dstndstn]

This is completely unrelated to large brick handling; I just hit it when I tried to write the test case for the large brick error. It seems cleaner to create a new PR for that bug.

…

On Wed, Sep 6, 2017 at 7:11 PM, Benjamin Alan Weaver < ***@***.***> wrote: ***@***.**** requested changes on this pull request. I don't know if you intend to include the fixes for large bricks in this PR, but at minimum you need to update the changes.rst file as well. ------------------------------ In py/desiutil/test/test_brick.py <#81 (comment)>: > @@ -251,6 +251,26 @@ def test_bricksize(self): self.assertEqual(B._bricks.bricksize, 0.25) B._bricks = None + def test_brick_radec_scalar(self): + """Test scalar to brick RA,Dec conversion. + """ + b = B.Bricks(bricksize=1.) + ra,dec = b.brick_radec(0., 0.) + self.assertEqual(ra, 0.5) + self.assertEqual(dec, 0.) + + def test_brick_radec_array(self): + """Test scalar to brick RA,Dec conversion. Please change the __doc__ string. You're testing an array, not a scalar. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#81 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBD_SwR3XycuZORD2LTfApz2EPCzWWxks5sfyaKgaJpZM4PO6nP> .

[weaverba137]

Fair enough. I'll merge as soon as the last checks pass.