Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

865 hi l1b spin phase #1102

Conversation

subagonsouth
Copy link
Contributor

@subagonsouth subagonsouth commented Oct 31, 2024

Change Summary

Overview

  • Adds Hi L1B function for populating direct event spin phase
  • I needed spin data for a different time than was hard coded into a fixture, so I modified the fixture to allow for inputting the start and end time of the spin table it generates.

Updated Files

  • imap_processing/hi/l1b/hi_l1b.py
    • Added function for populating L1B spin phase
  • imap_processing/tests/conftest.py
    • Renamed _set_spin_data_filepath to use_fake_spin_data_for_time and modified to allow configurable start and end time.
    • Modified generate_spin_data to accept and correctly use floating point MET times as input
    • Extracted use_test_spin_data_csv fixture so that a test spin phase file can be used if needed
  • imap_processing/tests/hi/test_data/l1a/imap_hi_l1a_45sensor-de_20250415_v000.cdf
    • The previous file had bad event_met times.
  • imap_processing/tests/hi/test_hi_l1b.py
    • Added test coverage for new L1B function
    • Added fixture to use fake spin data for full L1B DE test.
  • imap_processing/tests/spice/test_geometry.py
    • Updated test to use renamed/modified spin data fixture

Closes: #865

…rable spin start time

Add Hi L1B function for populating spin phase
Add test coverage for Hi L1B compute_instrument_spin_phase function
Update test using fake spin data fixture
@subagonsouth subagonsouth added the Ins: Hi Related to the IMAP-Hi instrument label Oct 31, 2024
@subagonsouth subagonsouth self-assigned this Oct 31, 2024
Copy link
Contributor

@tech3371 tech3371 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had one minor question. Otherwise, LGTM

"""Test coverage for compute_instrument_spin_phase."""
# set the get_instrument_spin_phase mock to return an array of values between
# 0 and 1
parse_sensor_number_mock.side_effect = lambda x, y: np.linspace(0, 1, len(x))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learn something new about .side_effect!

But this questions is a curiosity question. How is x, y used here? I thought parse_sensor_number() takes in only one argument. I see that you use this returned value to compare at line 185.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, I have my mock names mixed up. The argument order from decorators is reversed from the decorator order. This is actually setting the side_effect for the thing mocking get_instrument_spin_phase which is what I intended. And that function takes two arguments. I'll fix that.

@subagonsouth subagonsouth merged commit 8ac406c into IMAP-Science-Operations-Center:dev Nov 4, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ins: Hi Related to the IMAP-Hi instrument
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Hi l1b - spin phase
2 participants