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

raised error with message for negative tof value #1413

Merged
merged 5 commits into from
Dec 7, 2021

Conversation

TheBuffer
Copy link
Contributor

#1397 Added message to negative tof assert error

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @TheBuffer!

However, this makes the change only in the izzo algorithm (while we also have vallado). And besides, the concept of "orbit epochs" doesn't exist at this low level function, it's only the tof, so I think the error message might be misleading.

What about moving the check upwards, to the Maneuver.lambert classmethod?

@codecov
Copy link

codecov bot commented Dec 5, 2021

Codecov Report

Merging #1413 (23430f8) into main (aaa43d4) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1413   +/-   ##
=======================================
  Coverage   91.95%   91.95%           
=======================================
  Files          82       82           
  Lines        4348     4350    +2     
  Branches      426      427    +1     
=======================================
+ Hits         3998     4000    +2     
  Misses        260      260           
  Partials       90       90           
Impacted Files Coverage Δ
src/poliastro/maneuver.py 97.75% <100.00%> (+0.05%) ⬆️

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 aaa43d4...23430f8. Read the comment docs.

@astrojuanlu
Copy link
Member

Also, this will need a test.

Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Just a couple of minor things and we're ready!

src/poliastro/maneuver.py Outdated Show resolved Hide resolved
src/poliastro/core/iod.py Outdated Show resolved Hide resolved
@TheBuffer TheBuffer force-pushed the feature branch 2 times, most recently from 9fa77b6 to 225248f Compare December 7, 2021 06:05
Copy link
Member

@astrojuanlu astrojuanlu left a comment

Choose a reason for hiding this comment

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

Thanks a lot and congratulations for your first pull request! Merging

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