Repository: GeoscienceAustralia/GeodePy

Issue:
dec2hp and hp2... bug
Edge cases on some whole minute boundaries cause issues due to floating point precision:
`
>>>Python 3.7.4 (tags/v3.7.4:e09359112e, Jul  8 2019, 20:34:20) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from geodepy.angles import hp2dec, dec2hp
>>> dec2hp(1.083333333333333333333333)
1.046
>>> hp2dec(0.15)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\geodepy\angles.py", line 1024, in hp2dec
    raise ValueError(f'Invalid HP Notation: 3rd decimal place greater '        
ValueError: Invalid HP Notation: 3rd decimal place greater than 5: 0.15        
>>> hp2dec(20.2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\geodepy\angles.py", line 1024, in hp2dec
    raise ValueError(f'Invalid HP Notation: 3rd decimal place greater '
ValueError: Invalid HP Notation: 3rd decimal place greater than 5: 20.2
>>> hp2dec(20.02) 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\geodepy\angles.py", line 1024, in hp2dec
    raise ValueError(f'Invalid HP Notation: 3rd decimal place greater '
ValueError: Invalid HP Notation: 3rd decimal place greater than 5: 20.02
>>> hp2dec(133.4800) 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\geodepy\angles.py", line 1024, in hp2dec
    raise ValueError(f'Invalid HP Notation: 3rd decimal place greater '
ValueError: Invalid HP Notation: 3rd decimal place greater than 5: 133.48
>>>
`

Hints from issue comments:
@harry093: Fix by PR #157 

Solution patch:
diff --git a/geodepy/angles.py b/geodepy/angles.py
index 63ca274..49af0d8 100644
--- a/geodepy/angles.py
+++ b/geodepy/angles.py
@@ -934,11 +934,27 @@ def dec2hp(dec):
     :type dec: float
     :return: HP Notation (DDD.MMSSSS)
     :rtype: float
-    """
+    """      
     minute, second = divmod(abs(dec) * 3600, 60)
     degree, minute = divmod(minute, 60)
-    hp = degree + (minute / 100) + (second / 10000)
-    hp = round(hp, 16)
+
+    # floating point precision is 13 places for the variable 'dec' where values
+    # are between 256 and 512 degrees. Precision improves for smaller angles.
+    # In calculating the variable 'second' the precision is degraded by a factor of 3600 
+    # Therefore 'second' should be rounded to 9 DP and tested for carry.
+    if round(second, 9) == 60:
+        second = 0
+        minute += 1
+    
+    # to avoid precision issues with floating point operations
+    # a string will be built to represent a sexagesimal number and then converted to float
+    degree = f'{int(degree)}'
+    minute = f'{int(minute):02}'
+    second = f'{second:012.9f}'.rstrip('0').replace('.', '')
+    
+    hp_string = f'{degree}.{minute}{second}'
+    hp = float(hp_string)
+     
     return hp if dec >= 0 else -hp
 
 
@@ -1015,18 +1031,20 @@ def hp2dec(hp):
     """
     # Check if 1st and 3rd decimal place greater than 5 (invalid HP Notation)
     hp = float(hp)
-    hp_dec_str = f'{hp:.17f}'.split('.')[1]
-    if int(hp_dec_str[0]) > 5:
+    hp_deg_str, hp_mmss_str = f'{hp:.13f}'.split('.')
+    if int(hp_mmss_str[0]) > 5:
         raise ValueError(f'Invalid HP Notation: 1st decimal place greater '
                          f'than 5: {hp}')
-    if len(hp_dec_str) > 2:
-        if int(hp_dec_str[2]) > 5:
+    if len(hp_mmss_str) > 2:
+        if int(hp_mmss_str[2]) > 5:
             raise ValueError(f'Invalid HP Notation: 3rd decimal place greater '
                              f'than 5: {hp}')
-    degmin, second = divmod(abs(hp) * 1000, 10)
-    degree, minute = divmod(degmin, 100)
-    dec = degree + (minute / 60) + (second / 360)
-    dec = round(dec, 16)
+    # parse string to avoid precision problems with floating point ops and base 10 numbers
+    deg = abs(int(hp_deg_str))
+    min = int(hp_mmss_str[:2])
+    sec = float(hp_mmss_str[2:4] + '.' + hp_mmss_str[4:])
+    dec = sec / 3600 + min / 60 + deg
+
     return dec if hp >= 0 else -dec
 
 


Test patch:
diff --git a/geodepy/tests/test_angles.py b/geodepy/tests/test_angles.py
index 0a65957..e8d94fb 100644
--- a/geodepy/tests/test_angles.py
+++ b/geodepy/tests/test_angles.py
@@ -1,5 +1,6 @@
 import unittest
-from math import radians
+import os
+from math import radians, pi
 
 from geodepy.angles import (DECAngle, HPAngle, GONAngle, DMSAngle, DDMAngle,
                             dec2hp, dec2hpa, dec2gon, dec2gona,
@@ -85,6 +86,31 @@
 
 
 class TestConvert(unittest.TestCase):
+    def setUp(self):
+        self.testData = []
+        degreeValues = [0, 1, 2, 4, 8, 16, 32, 64, 128, 256]
+        dec_places = 13
+        error = 10**-(dec_places - 4)
+        for deg in degreeValues:
+            for min in range(60):
+                for sec in range(60):
+                    if sec:
+                        hp_minus = float(f'{deg:4d}.{min:02d}{sec-1:02d}' + '9' * (dec_places - 4))
+                        dec_minus = deg + (min / 60.0 + (sec - error) / 3600.0)
+                        gon_minus =  400.0 / 360.0 * dec_minus
+                        rad_minus = pi / 180.0 * dec_minus
+                        self.testData.append([hp_minus, dec_minus, gon_minus, rad_minus])
+                    hp = float(f'{deg:4d}.{min:02d}{sec:02d}')
+                    hp_plus = float(f'{deg:4d}.{min:02d}{sec:02d}' + '0' * (dec_places - 5) + '1')
+                    dec = deg + (min / 60.0 + sec / 3600.0)
+                    gon = 400.0 / 360.0 * dec
+                    rad = pi / 180.0 * dec
+                    self.testData.append([hp, dec, gon, rad])
+                    dec_plus = deg + (min / 60.0 + (sec + error) / 3600.0)
+                    gon_plus = 400.0 / 360.0 * dec_plus
+                    rad_plus = pi / 180.0 * dec_plus
+                    self.testData.append([hp_plus, dec_plus, gon_plus, rad_plus])
+
     def test_DECAngle(self):
         # Test DECAngle Methods
         for num, ex in enumerate(deca_exs):
@@ -465,6 +491,10 @@ def test_dec2hp(self):
         for num, ex in enumerate(hp_exs):
             self.assertAlmostEqual(ex, dec2hp(dec_exs[num]), 13)
             self.assertAlmostEqual(-ex, dec2hp(-dec_exs[num]), 13)
+        for check in self.testData:
+            hp, dec, gon, rad = check
+            self.assertAlmostEqual(hp, dec2hp(dec), 13)
+            self.assertAlmostEqual(-hp, dec2hp(-dec), 13)
 
     def test_dec2hpa(self):
         for num, ex in enumerate(dec_exs):
@@ -475,6 +505,10 @@ def test_dec2gon(self):
         for num, ex in enumerate(dec_exs):
             self.assertAlmostEqual(dec2gon(ex), gon_exs[num], 13)
             self.assertAlmostEqual(dec2gon(-ex), -gon_exs[num], 13)
+        for check in self.testData:
+            hp, dec, gon, rad = check
+            self.assertAlmostEqual(gon, dec2gon(dec), 13)
+            self.assertAlmostEqual(-gon, dec2gon(-dec), 13)
 
     def test_dec2gona(self):
         for num, ex in enumerate(dec_exs):
@@ -495,6 +529,13 @@ def test_hp2dec(self):
         for num, ex in enumerate(dec_exs):
             self.assertAlmostEqual(ex, hp2dec(hp_exs[num]), 13)
             self.assertAlmostEqual(-ex, hp2dec(-hp_exs[num]), 13)
+        for check in self.testData:
+            hp, dec, gon, rad = check
+            self.assertAlmostEqual(dec, hp2dec(hp), 13)
+            self.assertAlmostEqual(-dec, hp2dec(-hp), 13)
+
+        self.assertAlmostEqual(0, hp2dec(0), 13)
+        self.assertAlmostEqual(258, hp2dec(258), 13)
         self.assertAlmostEqual(hp2dec(hp_exs[0]) + hp2dec(hp_exs[1]),
                                dec_exs[0] + dec_exs[1], 13)
         # Test that invalid minutes and seconds components raise errors
@@ -523,6 +564,10 @@ def test_hp2gon(self):
         for num, ex in enumerate(hp_exs):
             self.assertAlmostEqual(hp2gon(ex), gon_exs[num], 13)
             self.assertAlmostEqual(hp2gon(-ex), -gon_exs[num], 13)
+        for check in self.testData:
+            hp, dec, gon, rad = check
+            self.assertAlmostEqual(gon, hp2gon(hp), 13)
+            self.assertAlmostEqual(-gon, hp2gon(-hp), 13)
 
     def test_hp2gona(self):
         for num, ex in enumerate(hp_exs):
@@ -531,8 +576,12 @@ def test_hp2gona(self):
 
     def test_hp2rad(self):
         for num, ex in enumerate(hp_exs):
-            self.assertEqual(hp2rad(ex), rad_exs[num])
-            self.assertEqual(hp2rad(-ex), -rad_exs[num])
+            self.assertAlmostEqual(hp2rad(ex), rad_exs[num], 15)
+            self.assertAlmostEqual(hp2rad(-ex), -rad_exs[num], 15)
+        for check in self.testData:
+            hp, dec, gon, rad = check
+            self.assertAlmostEqual(rad, hp2rad(hp), 15)
+            self.assertAlmostEqual(-rad, hp2rad(-hp), 15)
 
     def test_hp2dms(self):
         self.assertEqual(dms_ex.degree, hp2dms(hp_ex).degree)
@@ -552,6 +601,10 @@ def test_gon2dec(self):
         for num, ex in enumerate(gon_exs):
             self.assertAlmostEqual(gon2dec(ex), dec_exs[num], 14)
             self.assertAlmostEqual(gon2dec(-ex), -dec_exs[num], 14)
+        for check in self.testData:
+            hp, dec, gon, rad = check
+            self.assertAlmostEqual(dec, gon2dec(gon), delta = 5.8e-14)
+            self.assertAlmostEqual(-dec, gon2dec(-gon), delta = 5.8e-14)
 
     def test_gon2deca(self):
         for num, ex in enumerate(gon_exs):
@@ -562,6 +615,10 @@ def test_gon2hp(self):
         for num, ex in enumerate(gon_exs):
             self.assertEqual(gon2hp(ex), hp_exs[num])
             self.assertEqual(gon2hp(-ex), -hp_exs[num])
+        for check in self.testData:
+            hp, dec, gon, rad = check
+            self.assertAlmostEqual(hp, gon2hp(gon), 13)
+            self.assertAlmostEqual(-hp, gon2hp(-gon), 13)
 
     def test_gon2hpa(self):
         for num, ex in enumerate(gon_exs):
@@ -572,6 +629,10 @@ def test_gon2rad(self):
         for num, ex in enumerate(gon_exs):
             self.assertAlmostEqual(gon2rad(ex), rad_exs[num], 15)
             self.assertAlmostEqual(gon2rad(-ex), -rad_exs[num], 15)
+        for check in self.testData:
+            hp, dec, gon, rad = check
+            self.assertAlmostEqual(rad, gon2rad(gon), 13)
+            self.assertAlmostEqual(-rad, gon2rad(-gon), 13)
 
     def test_gon2dms(self):
         for num, ex in enumerate(gon_exs):