Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

__repr__ method of BaseRepresentation can fail #5889

Closed
StuartLittlefair opened this issue Mar 21, 2017 · 4 comments · Fixed by #5897
Closed

__repr__ method of BaseRepresentation can fail #5889

StuartLittlefair opened this issue Mar 21, 2017 · 4 comments · Fixed by #5897

Comments

@StuartLittlefair
Copy link
Contributor

StuartLittlefair commented Mar 21, 2017

The __repr__ method of the BaseRepresentation class uses a numpy view to access the values. It is possible to create objects for which this view cannot be taken, i.e (using v1.3.1):

from astropy.coordinates import EarthLocation, SkyCoord, get_moon
import astropy.units as u
from astropy.time import Time
import numpy as np

greenwich = EarthLocation.of_site('greenwich')
times = Time("2017-03-20T12:00:00") + np.linspace(-2,2,3)*u.hour
moon = get_moon(times, location=greenwich)
targets = SkyCoord([350.7*u.deg, 260.7*u.deg], [18.4*u.deg, 22.4*u.deg])
targs2d = targets[:, np.newaxis]
targs2d.transform_to(moon)

produces

Traceback (most recent call last):
  File "test_patch2.py", line 11, in <module>
    targs2d.transform_to(moon)
  File "/Users/sl/anaconda3/lib/python3.6/site-packages/astropy/coordinates/sky_coordinate.py", line 454, in transform_to
    return self.__class__(new_coord, **frame_kwargs)
  File "/Users/sl/anaconda3/lib/python3.6/site-packages/astropy/coordinates/sky_coordinate.py", line 212, in __init__
    kwargs = self._parse_inputs(args, kwargs)
  File "/Users/sl/anaconda3/lib/python3.6/site-packages/astropy/coordinates/sky_coordinate.py", line 360, in _parse_inputs
    .format(attr, coord_value, valid_kwargs[attr]))
  File "/Users/sl/anaconda3/lib/python3.6/site-packages/astropy/coordinates/representation.py", line 400, in __repr__
    arrstr = _array2string(self._values, prefix=prefixstr)
  File "/Users/sl/anaconda3/lib/python3.6/site-packages/astropy/coordinates/representation.py", line 229, in _values
    axis=-1).view(dtype).squeeze()
ValueError: new type not compatible with array.
@StuartLittlefair
Copy link
Contributor Author

StuartLittlefair commented Mar 21, 2017

Note that I have a fix in place for this by replacing this line with

# for complex shapes and structures, this may require a copy
try:
     return np.concatenate([coo.reshape(sh).value for coo in coordinates],
                           axis=-1).view(dtype).squeeze()
except:
    return np.concatenate([coo.reshape(sh).value for coo in coordinates],
                            axis=-1).astype(dtype).squeeze()

however, I'm struggling to add a regression test, since the example above runs into the exception reported in #5890 , which is thornier issue.

I cannot think of another way to create an N-D representation which cannot be accessed using a view like the one above.

@mhvk
Copy link
Contributor

mhvk commented Mar 21, 2017

Hmm, not good, that! Here is a simpler test:

CartesianRepresentation(np.arange(27).reshape(3,3,3), unit='m').T
# ... TypeError

@mhvk
Copy link
Contributor

mhvk commented Mar 21, 2017

My own sense would be to rewrite the code to something more robust, along the lines of

result = np.empty(self.shape, dtype)
for c, coo in zip(self.components, coordinates):
    result[str(c)] = coo.value
return result

I'll make a quick PR...

@mhvk
Copy link
Contributor

mhvk commented Mar 21, 2017

See #5896 for a fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants