# Add (double, double, units) constructor to SpherePoint and an associated getter

XMLWordPrintable

## Details

• Type: RFC
• Status: Implemented
• Resolution: Done
• Component/s:
• Labels:
None

## Description

It would be very convenient to have a SpherePoint constructor that takes a pair of doubles and a units argument. Coord has something similar and it is widely used, e.g. in unit tests. Specifically, I propose:

 SpherePoint(double long, double lat, AngleUnit units) 

This is intentionally different than Coord, which offers Coord(Point2D raDec, AngleUnit units=degrees). I prefer my proposed interface for several reasons:

• Point2D is intrinsically Cartesian, so I think it is a mistake to be able to construct a SpherePoint from one.
• In most cases where the Coord constructor is used, the values are specified directly, leading to this sort of clumsy code: coord = afwCoord.Coord(afwGeom.Point2D(45,0, 10.23), afwGeom.degrees). It is simpler to say point = afwGeom.SpherePoint(45.0, 10.23, afwGeom.degrees).
• Defaulting the units argument makes it difficult to tell what the code does, especially if RA and Dec are small. Furthermore, given our bias towards radians, I would expect that to be the default, if we had one.

Another proposed difference is that the units argument of the SpherePoint constructor always applies to both longitude and latitude. Coord usually does this, but treats the hours unit specially: RA is hours and Dec is degrees. I can see why that was done, but I think it is too clever and surprising to want to emulate it for SpherePoint, especially since the latter is not intrinsically RA, Dec anyway. I prefer simple consistency. If one really wants to provide hours and degrees one can provide the appropriate angles as arguments.

In addition, I propose a corresponding method to get the longitude and latitude of a SpherePoint as a pair of floats in the specified units: SpherePoint.getNumbers(units) -> std::pair<double, double>. Other name suggestions are welcome.

This is analogous to Coord.getPosition(units=degrees) with the same justification for the differences. I am less positive we need this (and am not thrilled with the name), but Coord.getPosition is fairly widely used, and it sometimes results in clearer code than retrieving each angle and extract the numeric value in the specified units.

## Activity

Hide
Krzysztof Findeisen added a comment -

Yes, I'll concede that that pattern of overloads is a bad idea.

Show
Krzysztof Findeisen added a comment - Yes, I'll concede that that pattern of overloads is a bad idea.
Hide
John Parejko added a comment -

I like getSphericalCoordinates(unit). Less confusing with getLongitude/getLatitude, and since SpherePoint is designed for points on the sky, it's pretty clear what you would get.

Show
John Parejko added a comment - I like getSphericalCoordinates(unit) . Less confusing with getLongitude/getLatitude , and since SpherePoint is designed for points on the sky, it's pretty clear what you would get.
Hide
Tim Jenness added a comment -

Where are we on this RFC?

Show
Tim Jenness added a comment - Where are we on this RFC?
Hide
Tim Jenness added a comment -

Ping.

Show
Tim Jenness added a comment - Ping.
Hide
Russell Owen added a comment -

Adopted as originally expressed but without a way to retrieve longitude and latitude as a pair of floats. The constructor is more important and there is no ambiguity about what it should look like. I am not sufficiently happy with any of the suggested names for methods to return a pair of floats to want to pick one at this time. If we find we really need it we can try another RFC.

Show
Russell Owen added a comment - Adopted as originally expressed but without a way to retrieve longitude and latitude as a pair of floats. The constructor is more important and there is no ambiguity about what it should look like. I am not sufficiently happy with any of the suggested names for methods to return a pair of floats to want to pick one at this time. If we find we really need it we can try another RFC.

## People

• Assignee:
Russell Owen
Reporter:
Russell Owen
Watchers:
John Parejko, Krzysztof Findeisen, Pim Schellart [X] (Inactive), Russell Owen, Tim Jenness