Skip to content

Removed orbital module#1976

Merged
abhisrkckl merged 2 commits intonanograv:masterfrom
dlakaplan:noorbital
Apr 2, 2026
Merged

Removed orbital module#1976
abhisrkckl merged 2 commits intonanograv:masterfrom
dlakaplan:noorbital

Conversation

@dlakaplan
Copy link
Copy Markdown
Contributor

#1974 : I don't know if this module is used. I suspect not. So delete to remove confusion.

@dlakaplan
Copy link
Copy Markdown
Contributor Author

So the tests that fail are in tests/test_kepler.py. but again I don't see that used elsewhere, so not clear if those tests should also be removed.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.25%. Comparing base (8499b93) to head (0a8635f).
⚠️ Report is 53 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1976      +/-   ##
==========================================
- Coverage   70.58%   70.25%   -0.34%     
==========================================
  Files         110      109       -1     
  Lines       25736    25556     -180     
  Branches     4054     4058       +4     
==========================================
- Hits        18167    17954     -213     
- Misses       6428     6459      +31     
- Partials     1141     1143       +2     

☔ View full report in Codecov by Sentry.
📢 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.

@dlakaplan dlakaplan added awaiting review This PR needs someone to review it so it can be merged labels Mar 27, 2026
@dlakaplan dlakaplan changed the title Removed orbital Removed orbital module Mar 27, 2026
@dlakaplan
Copy link
Copy Markdown
Contributor Author

@scottransom or @paulray : is this actually needed?

@scottransom
Copy link
Copy Markdown
Member

I don't think it is needed. I just looked at the history and I think it was intended as being the core of a pure new orbit model that Anne Archibald was writing. But that obviously never happened. It is probably fine to get rid of it, although it seems like nice code!

@dlakaplan
Copy link
Copy Markdown
Contributor Author

I don't think it is needed. I just looked at the history and I think it was intended as being the core of a pure new orbit model that Anne Archibald was writing. But that obviously never happened. It is probably fine to get rid of it, although it seems like nice code!

I have no particular animus, but it can be a little confusing. Could just add some "DEPRECATED" notes to the files instead.

@abhisrkckl
Copy link
Copy Markdown
Contributor

Yes, I think this is OK to remove. The code will be available in the git history if needed in the future.

@dlakaplan
Copy link
Copy Markdown
Contributor Author

OK, in that case I think you can merge this.

@abhisrkckl abhisrkckl merged commit 62ab2bc into nanograv:master Apr 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review This PR needs someone to review it so it can be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants