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

Pointing frame code #759

Merged

Conversation

laspsandoval
Copy link
Contributor

Change Summary

Overview

Provides algorithm that mocks Nick Duttons pointing frame (DPS frame) kernel generation. Note that I have TODOs that will be included in the next tickets.

New Files

  • pointing_frame_handler.py
    • writes a new C-kernel file containing the averaged pointing data

Testing

  • test_pointing_frame_handler.py
    • tests pointing_frame_handler.py
  • imap_science_0001.tf
    • pointing frame kernel used for test data
  • imap_sclk_0000.tsc
    • spacecraft clock kernel used for test data
  • imap_wkcp.tf
    • spacecraft frame kernel used for test data

@laspsandoval laspsandoval self-assigned this Aug 20, 2024
@greglucas greglucas added the SPICE Related to SPICE label Aug 20, 2024
Copy link
Contributor

@subagonsouth subagonsouth left a comment

Choose a reason for hiding this comment

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

This is a great start. I have some comments on organization and use of kernels.

imap_processing/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_pointing_frame_handler.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@subagonsouth subagonsouth left a comment

Choose a reason for hiding this comment

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

Ugg, sorry for requesting yet more changes. Thanks for sticking with this. I wonder if doing an early WIP PR would be helpful for critical code like this in the future?

imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/spice/pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_pointing_frame_handler.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_pointing_frame_handler.py Outdated Show resolved Hide resolved
Copy link
Contributor

@subagonsouth subagonsouth left a comment

Choose a reason for hiding this comment

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

Nice, Laura. This is looking great! I just have a few requests for minor changes 😬

imap_processing/tests/spice/test_kernels.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_kernels.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_kernels.py Outdated Show resolved Hide resolved
imap_processing/spice/kernels.py Outdated Show resolved Hide resolved
imap_processing/spice/kernels.py Show resolved Hide resolved
imap_processing/tests/spice/test_kernels.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_kernels.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_kernels.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_kernels.py Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_data/imap_spin.bc Outdated Show resolved Hide resolved
Copy link
Collaborator

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

Thank you for taking the time to address all of the feedback, I know there was a lot of it which can sometimes be hard to keep track of. I think this reads really nicely now, great work on this!

I just have a few minor nitpicks and I agree with all of Tim's comments so those should be addressed before merging too.

imap_processing/spice/kernels.py Outdated Show resolved Hide resolved
imap_processing/spice/kernels.py Outdated Show resolved Hide resolved
imap_processing/spice/kernels.py Outdated Show resolved Hide resolved
poetry.lock Outdated Show resolved Hide resolved
imap_processing/tests/spice/test_kernels.py Outdated Show resolved Hide resolved
@laspsandoval laspsandoval merged commit 8ef0cb6 into IMAP-Science-Operations-Center:dev Aug 27, 2024
17 checks passed
@laspsandoval laspsandoval deleted the dps_frame_transfer branch August 27, 2024 16:07
@greglucas
Copy link
Collaborator

Just as a note that we should discuss in a future tag-up. Because @subagonsouth had comments I think his approval should have been waited for. I approved suggesting my comments were sufficient, but thought maybe Tim should take another look before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SPICE Related to SPICE
Projects
Status: Done
Status: Done
3 participants