Skip to content

remove spice dependency from tests for updated lunarsky#620

Open
aelanman wants to merge 3 commits into
mainfrom
make_bland
Open

remove spice dependency from tests for updated lunarsky#620
aelanman wants to merge 3 commits into
mainfrom
make_bland

Conversation

@aelanman

Copy link
Copy Markdown
Contributor

Description

The latest version of lunarsky drops the dependency on spiceypy. As a result, the try blocks that catch SPICEUnknownFrameErrors in unit tests can be removed. This also bumps the minimum version of lunarsky.

Motivation and Context

This cleans up code that will no longer be needed.

Build or continuous integration change checklist:

  • If required or optional dependencies have changed (including version numbers), I have updated the readme to reflect this.
  • If this is a new CI setup, I have added the associated badge to the readme and to references/make_index.py (if appropriate).

@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (43a127e) to head (d1f4d93).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #620   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines         2455      2455           
=========================================
  Hits          2455      2455           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bhazelton

bhazelton commented Jun 19, 2026

Copy link
Copy Markdown
Member

@burdorfmitchell I think we'll need you to update the lunar reference sim data before we can merge this PR. The new lunarsky version is available on pypi.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: d1f4d93 Previous: a1f5f5f Ratio
tests/test_run_ref.py::test_run_sim[1.1_baseline_number] 0.3075095117381496 iter/sec (stddev: 0.09821328241750245) 0.4723100939736393 iter/sec (stddev: 0.08542779550019207) 1.54

This comment was automatically generated by workflow using github-action-benchmark.

@burdorfmitchell

Copy link
Copy Markdown
Contributor

The lunar reference simulation has been updated on the Brown Digital Repository.

@bhazelton

Copy link
Copy Markdown
Member

The lunar reference simulation has been updated on the Brown Digital Repository.

Thank you!!

@bhazelton

Copy link
Copy Markdown
Member

hmm, @aelanman it looks like it's passing the test for array equality but it's somewhat slower than the older version.

We can merge it as is and make an issue for speeding up lunarsky if you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants