From deca2f9c79988b39acacb216bc599fe5dfbd3b0b Mon Sep 17 00:00:00 2001
From: Brandon Rhodes <brandon@rhodesmill.org>
Date: Wed, 15 Dec 2021 13:18:12 -0500
Subject: [PATCH] Fix #673: subpoint_of() itrs_xyz vector was wrong

Also, fix a test that accidentally tested a position against itself.
---
 skyfield/tests/test_topos.py | 34 ++++++++++++++++++++--------------
 skyfield/toposlib.py         | 10 ++--------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/skyfield/tests/test_topos.py b/skyfield/tests/test_topos.py
index 0289fba5b..62bb86e58 100644
--- a/skyfield/tests/test_topos.py
+++ b/skyfield/tests/test_topos.py
@@ -200,6 +200,9 @@ def test_latlon_and_subpoint_methods(ts, angle):
     def check_lat(lat): assert abs(g.latitude.mas() - lat.mas()) < 0.1
     def check_lon(lon): assert abs(g.longitude.mas() - lon.mas()) < 0.1
     def check_height(h): assert abs(g.elevation.m - h.m) < 1e-7
+    def check_itrs(xyz, expected_distance):
+        actual_distance = length_of(g.itrs_xyz.m - xyz)
+        assert abs(actual_distance - expected_distance) < 1e-7
 
     lat, lon = wgs84.latlon_of(pos)
     check_lat(lat)
@@ -208,20 +211,23 @@ def check_height(h): assert abs(g.elevation.m - h.m) < 1e-7
     height = wgs84.height_of(pos)
     check_height(height)
 
-    g = wgs84.geographic_position_of(pos)
-    check_lat(g.latitude)
-    check_lon(g.longitude)
-    check_height(g.elevation)
-
-    g = wgs84.subpoint(pos)  # old deprecated method name
-    check_lat(g.latitude)
-    check_lon(g.longitude)
-    check_height(g.elevation)
-
-    g = wgs84.subpoint_of(pos)
-    check_lat(g.latitude)
-    check_lon(g.longitude)
-    assert g.elevation.m == 0.0
+    g2 = wgs84.geographic_position_of(pos)
+    check_lat(g2.latitude)
+    check_lon(g2.longitude)
+    check_height(g2.elevation)
+    check_itrs(g2.itrs_xyz.m, 0.0)
+
+    g2 = wgs84.subpoint(pos)  # old deprecated method name
+    check_lat(g2.latitude)
+    check_lon(g2.longitude)
+    check_height(g2.elevation)
+    check_itrs(g2.itrs_xyz.m, 0.0)
+
+    g2 = wgs84.subpoint_of(pos)
+    check_lat(g2.latitude)
+    check_lon(g2.longitude)
+    assert g2.elevation.m == 0.0
+    check_itrs(g2.itrs_xyz.m, 1234.0)
 
 def test_deprecated_position_subpoint_method(ts, angle):
     t = ts.utc(2018, 1, 19, 14, 37, 55)
diff --git a/skyfield/toposlib.py b/skyfield/toposlib.py
index 0997ae1f1..6ea6bb8db 100644
--- a/skyfield/toposlib.py
+++ b/skyfield/toposlib.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 
 from numpy import arctan2, array, array2string, cos, exp, sin, sqrt
-from .constants import ANGVEL, DAY_S, T0, pi, tau
+from .constants import ANGVEL, DAY_S, RAD2DEG, T0, pi, tau
 from .earthlib import refract
 from .framelib import itrs
 from .functions import (
@@ -230,13 +230,7 @@ def subpoint_of(self, position):
         """
         xyz_au, x, y, aC, R, lat = self._compute_latitude(position)
         lon = (arctan2(y, x) - pi) % tau - pi
-        return GeographicPosition(
-            latitude=Angle(radians=lat),
-            longitude=Angle(radians=lon),
-            elevation=Distance(lat * 0.0),
-            itrs_xyz=Distance(xyz_au),
-            model=self,
-        )
+        return self.latlon(lat * RAD2DEG, lon * RAD2DEG)
 
     def _compute_latitude(self, position):
         if position.center != 399: