Repository navigation
AltAz does not always broadcast across array args #5982
Description
The AltAz constructor broadcasts over some combinations of broadcastable shapes but not others:
def test_broadcast():
where = EarthLocation.from_geodetic(lat=45*u.deg, lon=30*u.deg, height=0*u.m)
for s1, s2 in (((1,),(1,)), ((2,),(1,)), ((1,),(2,)), ((2,),(2,)),
((2,1),(1,)), ((1,),(2,1)), ((2,1), (1,3))):
time = Time(np.full(np.prod(s1), 58000.).reshape(s1), format='mjd')
angle = np.full(np.prod(s2), 45.).reshape(s2) * u.deg
s12 = np.broadcast(time, angle).shape
try:
AltAz(alt=angle, az=angle, obstime=time, location=where)
status = 'OK'
except ValueError as e:
status = 'ValueError'
print s1, s2, s12, status
With astropy 1.3.2 and python 2.7.13 I get:
(1,) (1,) (1,) OK
(2,) (1,) (2,) ValueError
(1,) (2,) (2,) OK
(2,) (2,) (2,) OK
(2, 1) (1,) (2, 1) ValueError
(1,) (2, 1) (2, 1) OK
(2, 1) (1, 3) (2, 3) ValueError
ValueErrors are raised by coordinates.baseframe.__get__, e.g.
ValueError: attribute obstime should be scalar or have shape (1,), but is has shape (2,) and
could not be broadcast.
Possibly related to #5890?
Activity
mhvk commentedon Apr 25, 2017
@dkirkby - the broadcasting problems are somewhat "by design" -- that the shape of a coordinate be determined by its actual coordinates, not by the combination of coordinates and attributes. But we could revisit this...
dkirkby commentedon Apr 25, 2017
I view time as one of the coordinate axes, so found this behavior unexpected. Also since AltAz does broadcast nicely over other parameters, such as the observing wavelength, this behavior seems inconsistent.
In general, broadcasting over time like this can be quite useful, e.g., to map the (ra, dec) trajectory of the zenith during an observation.
mhvk commentedon Apr 26, 2017
In fairness to the current design choice, I think the kind of application that you note would be quite logically done by having, say, a given coordinate in ICRS, and then constructing a data-less
AltAzframe with a range of times, and then transforming to that frame. Indeed, your example works for:That said, I agree that at some level the current choice is arbitrary, and it would probably more logical if your example just worked as well. So, I'll label this as a feature request. (And a PR would be welcome....)
dkirkby commentedon Apr 26, 2017
I frequently use exactly this trick:
but the inverse (alt,az) -> (ra,dec) is also useful, e.g., in planning observations, hence this issue 😄
I am willing to try, but the
astropy.coordinatesimplementation is quite layered (understandably) and difficult to jump into. Are there any design docs that might help me get started?mhvk commentedon Apr 26, 2017
@dkirkby - yes, it took me a while to get into it too when I wrote the N-D stuff. The place that really matters in the initialization starts at https://github.com/astropy/astropy/blob/master/astropy/coordinates/baseframe.py#L649
As you see, that sets the data (possibly to
None), and then next all the attributes are checked against its shape if there is data, but the check for a consistent shape is postponed it there is no data. So, I think the solution might be to set the data later, after calculating a combined shape of all attributes and the data together (and possibly broadcasting the data).Note that all the checks are done by getting an attribute, and those checks are relative to
self.shape, whereshapeis defined here:https://github.com/astropy/astropy/blob/master/astropy/coordinates/baseframe.py#L745
p.s. An alternative approach would be to keep disallowing this, but allow
But the
SkyCoordinitialization is is a part of the code I find even more confusing (indeed, am trying to sanatize it...)mhvk commentedon Apr 26, 2017
p.p.s. I do worry a little about making the initialization even slower than it is already by trying to accept too much; but probably best to worry about that after seeing what it takes to make it work.
StuartLittlefair commentedon May 13, 2017
Late to the part as always but I share @mhvk 's concerns about making the
SkyCoordandBaseFrameclasses more complex than they already are.Also, couldn't one could achieve the use case stated by doing the broadcasting of angles and times yourself prior to initialising the
AltAzobject?Broadcast frames over both data and attributes
Add regression test for astropy#5982
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes
Broadcast frames over both data and attributes (#15121)