Ticket #21 (closed defect: fixed)

Opened 3 years ago

Last modified 7 months ago

sexagesimal bug

Reported by: perry Assigned to: somebody
Priority: normal Milestone: PyRAF 1.4 stsci_python 2.5
Component: none Version: 1.1.2
Severity: minor Keywords:
Cc:

Description

Hi Phil,

On Wed, 31 Mar 2004, Phil Hodge wrote:

In iraffunctions.py:

def clSexagesimal(d, m, s=0):

"""Convert d:m:s value to float""" return (d+(m+s/60.0)/60.0)

This does not work correctly for negative values of d. Also, since d, m, s are already float (rather than strings), it would be difficult to handle a value such as -0:18:30.

Do we actually recommend anywhere that people use this function directly? The parser does not include the minus sign in d, so this works in all cases within CL code. E.g., using the (little known) CL compatibility mode with a switch that echos the python equivalent of each CL line:

* --> clCompatibilityMode 1

Entering CL-compatibility (verbose) mode...

* cl>print(0:18:30)

0.308333333333 ----- Python ----- iraf.clPrint(iraf.clSexagesimal(0,18,30), _save=1) ------------------

* cl>print(-0:18:30)

-0.308333333333 ----- Python ----- iraf.clPrint( - iraf.clSexagesimal(0,18,30), _save=1) ------------------

(I put a * in front of the actual comments to make them easier to find.)

Oh, on poking around I do see one bug in the iraffunctions.py code: If you call real("0:18:30") it automatically does a call to clSexagesimal. But that call does *not* handle minus signs correctly -- it just does a split on ':' and converts each part to real. So the real() function should be fixed to handle minus signs correctly. Maybe this was somehow what you encountered?

Rick

Change History

03/03/08 11:57:15 changed by sontag

  • status changed from new to closed.
  • resolution set to fixed.
  • milestone set to PyRAF 1.4 stsci_python 2.5.

There seem to be two parts to this ticket: 1) whether clSexagesimal() is doing The Right Thing, and 2) whether calls like iraf.real("-0:18:30") are handled correctly.

Part 2 was fixed by Robert in r787 (ticket #61) which allows: iraf.real("-0:18:30"). This notices the sign and applies it to the result of the delegated call. 

--> iraf.real("-0:18:30")
-0.30833333333333335

As for Part 1, I would argue that the currently coded iraf.clSexagesimal() logic 

return (d+(m+s/60.0)/60.0)

actually is correct. For example, when one talks about "-0:18:30", they actually mean a call which translates to iraf.clSexagesimal(-0,-18,-30), which *does* produce the correct result. Assigning a negative sign to only *some* of the 3 arguments is a different thing (however non-intuitive it may be). There may be some users which calculate degrees, minutes, and seconds separately, for which they may have differing signs. Such users would rely on the current implementation. For example: 

--> iraf.clSexagesimal(-0,-18,-30)
-0.30833333333333335
--> iraf.clSexagesimal(0,18,-30)
0.29166666666666669