Support telescope position from EarthLocations for SubarrayDescription
#2424
Conversation
StFroese
force-pushed
the
support_earth_location
branch
from
25fc030
to
cdd9b8d
Compare
StFroese
force-pushed
the
support_earth_location
branch
from
cdd9b8d
to
3acdd9b
Compare
|
Ignore the test for now, I'll write a new one as a regression test... |
| tel_positions : Dict[int, np.ndarray] | ||
| dict of x,y,z telescope positions on the ground by tel_id. These are | ||
| tel_positions : Dict[int, Union[np.ndarray, EarthLocation]] | ||
| dict of x,y,z telescope positions on the ground by tel_id or EarthLocation. These are |
There was a problem hiding this comment.
by tel_id or EarthLocation sounds like EarthLocation could be the key of the dictionary, not another format for the value.
There was a problem hiding this comment.
True, better with something like
dict of telescope positions by tel_id, given as x,y,z position or EarthLocation.
| @@ -91,7 +91,9 @@ def __init__( | |||
| coordinate system used for `tel_positions`. | |||
| """ | |||
| self.name = name | |||
| self.positions = tel_positions or dict() | |||
| self.positions: Dict[int, Union[np.ndarray, EarthLocation]] = ( | |||
There was a problem hiding this comment.
We shouldn't use mixed types for internal storage. We should accept EarthLocation or GroundFrame coordinates for the inputs, but then convert in __init__ to one common format.
I think for now, that should be the previous, so ground frame coordinas.
| pos_x = [p[0].to_value(u.m) for p in self.positions.values()] | ||
| pos_y = [p[1].to_value(u.m) for p in self.positions.values()] | ||
| pos_z = [p[2].to_value(u.m) for p in self.positions.values()] | ||
| positions = self.positions.values() |
There was a problem hiding this comment.
See above, internal representation should not be mixed.
fixes #2162