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

Earth atmosphere computations to core #1280

Merged
merged 8 commits into from Jul 17, 2021

Conversation

Yash-10
Copy link
Member

@Yash-10 Yash-10 commented Jul 17, 2021

The jacchia module had a lot of computations inside the high-level module. Here, the attempt is to just move jacchia and some more computations to core.

  • I am wondering if there would already be an in-built method that does the equivalent of _get_index
  • It would have been great if _check_altitude method could also be moved to core but the f-strings in the ValueError condition seemed to prohibit it.

Thanks!

@codecov
Copy link

codecov bot commented Jul 17, 2021

Codecov Report

Merging #1280 (417e88f) into main (c2acd4c) will increase coverage by 0.00%.
The diff coverage is 95.13%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1280   +/-   ##
=======================================
  Coverage   91.88%   91.89%           
=======================================
  Files          79       80    +1     
  Lines        4167     4196   +29     
  Branches      362      360    -2     
=======================================
+ Hits         3829     3856   +27     
- Misses        256      258    +2     
  Partials       82       82           
Impacted Files Coverage Δ
src/poliastro/earth/atmosphere/coesa76.py 98.26% <66.66%> (ø)
src/poliastro/earth/atmosphere/jacchia.py 92.10% <71.42%> (-5.66%) ⬇️
src/poliastro/earth/atmosphere/base.py 90.00% <90.90%> (+3.15%) ⬆️
src/poliastro/core/earth_atmosphere/util.py 78.26% <91.66%> (ø)
src/poliastro/core/earth_atmosphere/jacchia.py 98.19% <98.19%> (ø)

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 c2acd4c...417e88f. Read the comment docs.

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.

Looks good to me! Documentation is failing because of this:

sphinx.errors.SphinxWarning: Cannot resolve import of unknown module poliastro.core.earth_atmosphere.util in poliastro.earth.atmosphere.base

src/poliastro/earth/atmosphere/base.py Show resolved Hide resolved
@astrojuanlu astrojuanlu merged commit ea5bf1e into poliastro:main Jul 17, 2021
@astrojuanlu
Copy link
Member

Merging, thanks @Yash-10 !

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