wlandsman / IDLAstro

Astronomy related procedures in the commercial IDL language
https://asd.gsfc.nasa.gov/archive/idlastro/
BSD 2-Clause "Simplified" License
144 stars 64 forks source link

Inconsistent behavior of ten #6

Closed giordano closed 8 years ago

giordano commented 8 years ago

ten function has an inconsistent behavior between numeric and string input, consider the following:

GDL> print, ten(-5.0, -60.0, -3600.0), ten("-5.0:-60.0:-3600.0")
      -7.0000000      -3.0000000
GDL> print, ten(-0.0, 60.0), ten("-0.0:60.0") 
       1.0000000      -1.0000000

I know that negative inputs are a problem (and negative zero in particular), but the algorithm used in the case of numerical input seems to be rather inconsistent (see the first line). The current algorithm for the numeric case is:

sign = sign(deg)
return sign*(abs(deg) + abs(min)/60.0 + abs(sec)/3600.0)

I would suggest to use the same algorithm used in the string case

sign = deg >= 0 ? 1 : 1
return sign*(abs(deg) + min/60.0 + sec/3600.0)

or even (for both numeric and string input)

degsign = deg >= 0 ? 1 : 1
minsign = min >= 0 ? 1 : 1
return degsign*(abs(deg) + minsign(abs(min)/60.0 + sec/3600.0))

in order to make ten("0:-0:60") conceptually consistent with the behavior of ten("-0:60:0"). I can't find an authoritative source stating which is the "correct", or at least the most commonly accepted, algorithm for sexagesimal to decimal conversion (especially with regard to dealing with signs of degrees, minutes, and seconds).

Of course, these change, the last suggested one in particular, raise backward compatibility concerns, I don't know the policy of the project in this regard.

wlandsman commented 8 years ago

ten function has an inconsistent behavior between numeric and string input, consider the following:

GDL> print, ten(-5.0, -60.0, -3600.0), ten("-5.0:-60.0:-3600.0") -7.0000000 -3.0000000

I believe the string input is giving incorrect results. As the documentation states, "A minus sign on any nonzero element of the input vector causes all the elements to be < 0."

But I do not think it worth changing. I will add to the documentation a note about the different behaviour of string input.