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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reformat on atmospheres #756

Merged
merged 5 commits into from Aug 17, 2019
Merged

Reformat on atmospheres #756

merged 5 commits into from Aug 17, 2019

Conversation

@jorgepiloto
Copy link
Member

jorgepiloto commented Aug 14, 2019

This pull request reformats the actual poliastro.atmosphere module by including the following features:

  • Parameters for each atmosphere have been stored in atmosphere/data/*.dat by making use of Astropy capabilities, which translates into a cleaner implementation in each *.py file.

  • Pure atmospheric models have been partially or fully implemented by making use of outdated official paper constants.

  • Values are easier to modify and read since each line of the data files has the same format as official tables.

I am opening this as a draft since I still need to:

  • ⚠️ Modify setup.cfg and to install *.dat files. ⚠️
  • More tests, so coverage is not decreased.
  • More documentation on atmospheres.
  • Test if this solves #694
@jorgepiloto jorgepiloto mentioned this pull request Aug 14, 2019
2 of 2 tasks complete
@jorgepiloto jorgepiloto force-pushed the jorgepiloto:coesa branch 2 times, most recently from 3057450 to b53e1c9 Aug 15, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #756 into master will increase coverage by 1.5%.
The diff coverage is 91.24%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #756     +/-   ##
=========================================
+ Coverage   87.57%   89.08%   +1.5%     
=========================================
  Files          57       60      +3     
  Lines        3252     3316     +64     
  Branches      279      274      -5     
=========================================
+ Hits         2848     2954    +106     
+ Misses        328      287     -41     
+ Partials       76       75      -1
Impacted Files Coverage Δ
src/poliastro/constants/general.py 100% <ø> (ø) ⬆️
src/poliastro/atmosphere/__init__.py 100% <100%> (ø)
src/poliastro/atmosphere/util.py 63.63% <70%> (-36.37%) ⬇️
src/poliastro/atmosphere/base.py 80% <80%> (ø)
src/poliastro/atmosphere/coesa62.py 91.25% <91.25%> (ø)
src/poliastro/atmosphere/coesa76.py 95.19% <95.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 8675f04...4ab2f1b. Read the comment docs.

@jorgepiloto jorgepiloto marked this pull request as ready for review Aug 16, 2019
@Juanlu001 Juanlu001 self-requested a review Aug 16, 2019
Copy link
Member

Juanlu001 left a comment

Brilliant job! From my side this is ready to merge, no need to wait until the validation although we can do it if you prefer.

@jorgepiloto jorgepiloto force-pushed the jorgepiloto:coesa branch from b53e1c9 to 4ab2f1b Aug 17, 2019
@Juanlu001 Juanlu001 merged commit fb4886a into poliastro:master Aug 17, 2019
8 of 9 checks passed
8 of 9 checks passed
codeclimate 8 issues to fix
Details
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_py36 Your tests passed on CircleCI!
Details
ci/circleci: test_py37 Your tests passed on CircleCI!
Details
codecov/patch 91.24% of diff hit (target 87.57%)
Details
codecov/project 89.08% (+1.5%) compared to 8675f04
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@Juanlu001

This comment has been minimized.

Copy link
Member

Juanlu001 commented Aug 17, 2019

👏 Merged!

@jorgepiloto jorgepiloto deleted the jorgepiloto:coesa branch Aug 17, 2019
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’t perform that action at this time.