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

transform_to(AltAz) de-scalarizes if input has a distance #3920

Closed
eteq opened this issue Jul 7, 2015 · 1 comment · Fixed by #4039
Closed

transform_to(AltAz) de-scalarizes if input has a distance #3920

eteq opened this issue Jul 7, 2015 · 1 comment · Fixed by #4039
Assignees
Milestone

Comments

@eteq
Copy link
Member

eteq commented Jul 7, 2015

Surprising behavior noted (indirectly) by @bmorris3 :

>>> aa = AltAz(location=EarthLocation.from_geodetic(0*u.deg, 0*u.deg, 0),obstime=Time('2015-1-1'))
>>> m=SkyCoord(10*u.deg,3*u.deg)
>>> m.transform_to(aa).shape
()

That part makes sense: the input is a scalar so the output is too

>>> aa = AltAz(location=EarthLocation.from_geodetic(0*u.deg, 0*u.deg, 0),obstime=Time('2015-1-1'))
>>> m=SkyCoord(10*u.deg,3*u.deg, 1*u.AU)
>>> m.transform_to(aa).shape
(1,)

oops! For some reason it decided to switch to a length-1 array coordinate. Definitely should still be a scalar.

This is definitely a bug and not at all intended, but changing it is breaking backwards-compatibility. How do we want to address this? Deprecation? Or leave it in for 1.0.x but change it for 1.1.x?

I'm not sure how used this functionality is right now, so it may not be an issue to slip in the change...

cc @adrn @astrofrog

@eteq eteq self-assigned this Jul 7, 2015
@eteq eteq added this to the v1.1.0 milestone Jul 7, 2015
@astrofrog
Copy link
Member

I don't think we really need to preserve backward compatibility for buggy behavior like this. But can you give an example of realistic code that would break because of this assumption to see what we can do?

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

Successfully merging a pull request may close this issue.

3 participants