Repository navigation
Improved handling of longitude wrap angles in frames - #3223
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
Cadair
left a comment
There was a problem hiding this comment.
This looks like a clever way of doing it. Presuming that represent_as is always called when we need it to be?
| If a frame wrap angle is set, use that wrap angle for any spherical representations. | ||
| """ | ||
| data = super().represent_as(base, s, in_frame_units=in_frame_units) | ||
| if self._wrap_angle is not None and \ |
There was a problem hiding this comment.
line break using () rather than \?
There was a problem hiding this comment.
I originally had it that way, but when using the (, the way that I want to indent the following line becomes exactly 4 spaces, so it's easy to confuse for an actual indention (and the bot complained as well). So, I changed it to the \ so that I could indent the folllowing line only 3 spaces and break that confusion. It's admittedly silly. How would you indent it?
There was a problem hiding this comment.
Fair enough. I just have a pathological hatred for \ lol
|
This fixes the 32bit build error! |
It bears repeating in this PR that the cleverness is not mine; I copied it from here: astropy/astropy#7462 (comment). As far as I've seen, yes, |
(This was originally part of #3212, but I feel that it qualifies as a bug fix to get into 1.0.2 rather than wait until 1.1.)
We intend to have all of our frames wrap longitudes at 180 degrees (i.e., the range is -180 deg to +180 deg, rather than 0 deg to +360 deg). Prior to this PR, the longitude wrap angle would get set to the desired value if a frame was created with a
(Unit)SphericalRepresentation. However, that meant that if the frame was created with aCartesianRepresentation, and then converted to aSphericalRepresentation, the longitude wrap angle would not get set. This happens frequently after frame transformations, such as forAIAMap.observer_coordinate, which gets converted from HEC coordinates and often shows HGS longitudes of ~359.99 deg rather than a small negative number.This PR takes a different approach. Rather than modifying the representation as it goes into the frame, the approach is to modify the representation as it comes out. That is, the internal data can be whatever, but whenever the user asks for it as a
(Unit)SphericalRepresentation, it is provided with the desired wrap angle.