Skip to content
This repository has been archived by the owner on Oct 14, 2023. It is now read-only.

poliastro.twobody.orbit cleanup #1430

Merged
merged 15 commits into from
Dec 27, 2021

Conversation

astrojuanlu
Copy link
Member

Generally working towards #1425, but did a lot of other things along the way. Each commit could be reviewed separately.

Summary:

  • Remove almost all instances of np.dot and replace them by @ finally!
  • Remove Orbit.from_body_ephem at last!
  • Move some logic of Orbit classmethods to separate functions
  • Rename and move some stuff around for consistency

Managed to remove ~200 lines from orbit.py, but we can do much more. Orbit.frozen in particular is a bit daunting.

@codecov
Copy link

codecov bot commented Dec 20, 2021

Codecov Report

Merging #1430 (b95200a) into main (c947781) will increase coverage by 0.45%.
The diff coverage is 91.36%.

❗ Current head b95200a differs from pull request most recent head 22c4a61. Consider uploading reports for the commit 22c4a61 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1430      +/-   ##
==========================================
+ Coverage   91.67%   92.13%   +0.45%     
==========================================
  Files          82       90       +8     
  Lines        4350     4375      +25     
  Branches      427      421       -6     
==========================================
+ Hits         3988     4031      +43     
+ Misses        272      256      -16     
+ Partials       90       88       -2     
Impacted Files Coverage Δ
src/poliastro/bodies.py 85.18% <0.00%> (ø)
src/poliastro/plotting/interactive.py 78.62% <ø> (ø)
src/poliastro/io.py 34.37% <34.37%> (ø)
src/poliastro/plotting/_base.py 84.83% <75.00%> (ø)
src/poliastro/core/propagation/vallado.py 87.09% <87.09%> (ø)
src/poliastro/twobody/elements.py 90.47% <88.88%> (-2.39%) ⬇️
src/poliastro/core/propagation/mikkola.py 92.72% <92.72%> (ø)
src/poliastro/core/propagation/gooding.py 93.75% <93.75%> (ø)
src/poliastro/core/propagation/danby.py 96.49% <96.49%> (ø)
src/poliastro/core/elements.py 100.00% <100.00%> (ø)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c947781...22c4a61. Read the comment docs.

@astrojuanlu
Copy link
Member Author

Pushed 1 more commit to finish the work we started on #1026 and separate the propagators in modules.

I promise I won't add anything else :) Next I'd love to tackle #921 and #1364.

Merry Christmas! 🎄

@astrojuanlu
Copy link
Member Author

I promise I won't add anything else :) Next I'd love to tackle #921 and #1364.

Next in line: astrojuanlu#12

@jorgepiloto
Copy link
Member

Reviewing this 🔎

Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

This was a heavy PR but many things got simplified. Amazing work @astrojuanlu 👏🏽

src/poliastro/core/elements.py Show resolved Hide resolved


@jit
def circular_velocity(k, a):
Copy link
Member

Choose a reason for hiding this comment

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

Which is the reason behind shipping this to poliatro.core.elements?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using that module for small Astrodynamics formulas, I think it fits there better. Probably we should find a better module name for the other functions in the future.

@astrojuanlu astrojuanlu merged commit fd15967 into poliastro:main Dec 27, 2021
@astrojuanlu
Copy link
Member Author

Thanks a lot @jorgepiloto ! 🚀

@astrojuanlu astrojuanlu deleted the twobody-orbit-cleanup branch December 27, 2021 12:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants