Pr 5891 revised -- fix multidimensional skycoord problems #5897
Conversation
|
Note that this just fixes Also, I had started on revamping the parsing (which presently is very hard to understand), so maybe it is best to leave that for another time (see #5751 (comment)) |
|
Can't fault this for elegance @mhvk. It doesn't look like there's any significant performance hit for large representations either. :+1 from me. |
| return np.concatenate([coo.reshape(sh).value for coo in coordinates], | ||
| axis=-1).view(dtype).squeeze() | ||
| # For PY2, convert possible unicode literal to string | ||
| coo_items = [(str(c) if six.PY2 else c, getattr(self, c)) |
There was a problem hiding this comment.
Any reason not to move if six.PY2 out of the loop? Besides making the loop (slightly) faster it would be easier to remove it when python2 support is dropped.
There was a problem hiding this comment.
It used to do str(c) unconditionally which is guaranteed to work, and really the only reason I put this here was to be sure we would clean it up correctly for astropy 3.0. But on second thought, probably anybody doing that would do a grep for PY2, so then this would be found anyway.
Let me change it back...
There was a problem hiding this comment.
actually, on second thought, I wonder if this was just for numpy 1.6. I'll try without and see if travis signals any problems.
There was a problem hiding this comment.
seems to fail, so it's a python 2 thing. :(
|
seems to fail, so it's a python 2 thing. :(
OK, I just use `str(c)` now and adjusted the comment.
|
|
OK, tests now all pass. |
|
Merging since everybody seemed happy with this. |
Pr 5891 revised -- fix multidimensional skycoord problems
Pr 5891 revised -- fix multidimensional skycoord problems
fixes #5889, #5890
replaces #5891 with a simpler implementation.