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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lambert becomes Maneuver #680

Merged
merged 2 commits into from Jun 10, 2019
Merged

Lambert becomes Maneuver #680

merged 2 commits into from Jun 10, 2019

Conversation

@jorgepiloto
Copy link
Member

jorgepiloto commented May 27, 2019

This tries to solve for #114. I added the option to pass as an argument a lambert solver. The idea was taken from the method propagate in the Orbit class. Just a simple change in porkchops and they keep working without problems.

Some things still need to be done:

  • Correct notebooks and documentation
  • When adding new tests, should we move the ones from iod module to the maneuver one? Or do we implement new ones with the same data?

But before correcting them I just wanted to be sure if you think this is the way on solving this 馃殌

@jorgepiloto jorgepiloto force-pushed the jorgepiloto:lambert branch from 204ac25 to 5dcbaee May 27, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented May 27, 2019

Codecov Report

Merging #680 into master will increase coverage by <.01%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   87.09%   87.09%   +<.01%     
==========================================
  Files          53       53              
  Lines        2634     2650      +16     
  Branches      232      233       +1     
==========================================
+ Hits         2294     2308      +14     
- Misses        266      267       +1     
- Partials       74       75       +1
Impacted Files Coverage 螖
src/poliastro/plotting/porkchop.py 77.04% <100%> (+1.61%) 猬嗭笍
src/poliastro/maneuver.py 93.02% <83.33%> (-1.58%) 猬囷笍

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 8c3fdfc...06e46ac. Read the comment docs.

Copy link
Member

Juanlu001 left a comment

This looks very good @jorgepiloto!

While we are at this, do you think we can take care of #495?

@jorgepiloto

This comment has been minimized.

Copy link
Member Author

jorgepiloto commented Jun 3, 2019

This looks very good @jorgepiloto!

While we are at this, do you think we can take care of #495?

Sure! I also saw #348. I will start working on all of them this afternoon 馃憤

@jorgepiloto jorgepiloto force-pushed the jorgepiloto:lambert branch from 5dcbaee to 188d641 Jun 9, 2019
Apply black
@jorgepiloto jorgepiloto force-pushed the jorgepiloto:lambert branch from 188d641 to 06e46ac Jun 9, 2019

if tof <= 0:
return None, None, None, None, None

try:
(v_dpt, v_arr), = lambert(Sun.k, rr_dpt_body.xyz, rr_arr_body.xyz, tof)
# Lambert is now a Maneuver object
man_lambert = Maneuver.lambert(ss_dpt, ss_arr)

This comment has been minimized.

Copy link
@Juanlu001

Juanlu001 Jun 10, 2019

Member

Much nicer!

Copy link
Member

Juanlu001 left a comment

This is good to go 馃槏

@Juanlu001 Juanlu001 merged commit 5416d2d into poliastro:master Jun 10, 2019
10 checks passed
10 checks passed
ci/circleci: coverage Your tests passed on CircleCI!
Details
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: quality Your tests passed on CircleCI!
Details
ci/circleci: test_py35 Your tests passed on CircleCI!
Details
ci/circleci: test_py36 Your tests passed on CircleCI!
Details
ci/circleci: test_py37 Your tests passed on CircleCI!
Details
codeclimate All good!
Details
codecov/patch 90.9% of diff hit (target 87.09%)
Details
codecov/project 87.09% (+<.01%) compared to 8c3fdfc
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can鈥檛 perform that action at this time.