1020 bug get spacecraft spin phase always returns nans

imap_processing/spice/geometry.py

@@ -145,7 +145,7 @@ def get_spin_data() -> pd.DataFrame:
         # Handle the case where the environment variable is not set
         raise ValueError("SPIN_DATA_FILEPATH environment variable is not set.")
 
-    spin_df = pd.read_csv(path_to_spin_file)
+    spin_df = pd.read_csv(path_to_spin_file, comment="#")
     # Combine spin_start_sec and spin_start_subsec to get the spin start
     # time in seconds. The spin start subseconds are in milliseconds.
     spin_df["spin_start_time"] = (
@@ -197,8 +197,8 @@ def get_spacecraft_spin_phase(
     # array([0, 15, 30, 45, 60])
     # >>> np.searchsorted(df['a'], [0, 13, 15, 32, 70], side='right')
     # array([1, 1, 2, 3, 5])
-    last_spin_indices = np.searchsorted(
-        spin_df["spin_start_time"], query_met_times, side="right"
+    last_spin_indices = (
+        np.searchsorted(spin_df["spin_start_time"], query_met_times, side="right") - 1
     )
     # Make sure input times are within the bounds of spin data
     spin_df_start_time = spin_df["spin_start_time"].values[0]
@@ -207,7 +207,7 @@ def get_spacecraft_spin_phase(
     )
     input_start_time = query_met_times.min()
     input_end_time = query_met_times.max()
-    if input_start_time < spin_df_start_time or input_end_time > spin_df_end_time:
+    if input_start_time < spin_df_start_time or input_end_time >= spin_df_end_time:
         raise ValueError(
             f"Query times, {query_met_times} are outside of the spin data range, "
             f"{spin_df_start_time, spin_df_end_time}."

imap_processing/tests/spice/test_data/fake_spin_data.csv

@@ -0,0 +1,19 @@
+# This is a fake csv file for the sole purpose of testing the get_spacecraft_spin_phase function
+spin_number,spin_start_sec,spin_start_subsec,spin_period_sec,spin_period_valid,spin_phase_valid,spin_period_source,thruster_firing
+# Start with thruster firing
+0,0,0,15.0,1,1,0,1
+# Turn thruster firing off then 3 valid spins
+1,15,0,15.0,1,1,0,0
+2,30,0,15.0,1,1,0,0
+3,45,0,15.0,1,1,0,0
+# Missing spin then 2 good spins
+5,75,0,15.0,1,1,0,0
+6,90,0,15.0,1,1,0,0
+# invalid spin period
+7,105,0,15.0,0,1,0,0
+# invalid spin phase
+8,120,0,15.0,1,0,0,0
+# 1 good spin
+9,135,0,15.0,1,1,0,0
+# Thruster firing on
+10,150,0,15.0,1,1,0,1

imap_processing/tests/spice/test_geometry.py

@@ -41,51 +41,60 @@ def test_imap_state_ecliptic(use_test_metakernel):
     assert state.shape == (6,)
 
 
-@pytest.mark.usefixtures("_set_spin_data_filepath")
+@pytest.fixture()
+def fake_spin_data(monkeypatch, spice_test_data_path):
+    """Generate fake spin dataframe for testing"""
+    fake_spin_path = spice_test_data_path / "fake_spin_data.csv"
+    monkeypatch.setenv("SPIN_DATA_FILEPATH", str(fake_spin_path))
+    return fake_spin_path
+
+
 @pytest.mark.parametrize(
-    "query_met_times, expected_type, expected_length",
+    "query_met_times, expected",
     [
-        (453051323.0, float, None),  # Scalar test
-        (np.array([453051323.0, 453051324.0]), float, 2),  # Array test
-        (np.array([]), None, 0),  # Empty array test
-        (np.array([453051323.0]), float, 1),  # Single element array test
-        # 452995203.0 is a midnight time which should have invalid spin
-        # phase and period flags on in the spin data file. The spin phase
-        # should be invalid.
-        (452995203.0, np.nan, None),
-        # Test that five minutes after midnight is also invalid since
-        # first 10 minutes after midnight are invalid.
-        (np.arange(452995203.0, 452995203.0 + 300), np.nan, 300),
+        (15, 0.0),  # Scalar test
+        (np.array([15.1, 30.1]), np.array([0.1 / 15, 0.1 / 15])),  # Array test
+        (np.array([]), None),  # Empty array test
+        (np.array([50]), np.array([5 / 15])),  # Single element array test
+        # The first spin has thruster firing set, but should return valid value
+        (5.0, 5 / 15),
+        # Test invalid spin period flag causes nan
+        (106.0, np.nan),
+        # Test invalid spin phase flag causes nans
+        (np.array([121, 122, 123]), np.full(3, np.nan)),
+        # Test that invalid spin period causes nans
+        (np.array([110, 111]), np.full(2, np.nan)),
+        # Test for time in missing spin
+        (65, np.nan),
+        (np.array([65.1, 66]), np.full(2, np.nan)),
+        # Combined test
         (
-            [453011323.0],
-            np.nan,
-            1,
-        ),  # Test for spin phase that's outside of spin phase range
-        (
-            453011323.0,
-            np.nan,
-            None,
-        ),  # Test for spin phase that's outside of spin phase range
+            np.array([7.5, 30, 61, 75, 106, 121, 136]),
+            np.array([0.5, 0, np.nan, 0, np.nan, np.nan, 1 / 15]),
+        ),
     ],
 )
-def test_get_spacecraft_spin_phase(query_met_times, expected_type, expected_length):
+def test_get_spacecraft_spin_phase(query_met_times, expected, fake_spin_data):
     """Test get_spacecraft_spin_phase() with generated spin data."""
     # Call the function
     spin_phases = get_spacecraft_spin_phase(query_met_times=query_met_times)
 
-    # Check the type of the result
-    if expected_type is np.nan:
-        assert np.isnan(spin_phases).all(), "Spin phase must be NaN."
-    elif isinstance(expected_type, float):
-        assert isinstance(spin_phases, float), "Spin phase must be a float."
-
-    # If the expected length is None, it means we're testing a scalar
-    if expected_length is None:
+    # Test the returned type
+    if isinstance(expected, float):
         assert isinstance(spin_phases, float), "Spin phase must be a float."
+    elif expected is None:
+        assert len(spin_phases) == 0, "Spin phase must be empty."
     else:
-        assert (
-            len(spin_phases) == expected_length
-        ), f"Spin phase must have length {expected_length} for array input."
+        assert spin_phases.shape == expected.shape
+    # Test the value
+    np.testing.assert_array_almost_equal(spin_phases, expected)
+
+
+@pytest.mark.parametrize("query_met_times", [-1, 165])
+def test_get_spacecraft_spin_phase_value_error(query_met_times, fake_spin_data):
+    """Test get_spacecraft_spin_phase() for raising ValueError."""
+    with pytest.raises(ValueError, match="Query times"):
+        _ = get_spacecraft_spin_phase(query_met_times)
 
 
 @pytest.mark.usefixtures("_set_spin_data_filepath")