Add some more tests and checks

[Yash-10]

This pull request is a continuation of addition of some tests and checks.

• The altitude checks were added to prevent creation of an orbit with negative altitude. This might yield unexpected results at some places, for example, in the lithobrake event.

Questions

• It looks like @pytest.mark.skipif("czml3" not in sys.modules, reason="requires czml3") gets repeated for each test in test_czml.py. Could we instead have a global pytestmark to skip the whole module if czml3 is not installed?

pytestmark = pytest.mark.skipif("czml3" not in sys.modules)

• ALL_PROPAGATORS in twobody/propagation.py seem to not contain markley and danby. This seems to be contrary to the name of the variable. Is this expected? If not, should we include all the propagators in ALL_PROPAGATORS?

• In Some test additions #1253, you had advised to make expected_doc as a fixture. It seems that there are more expected_doc's inside the tests. Is it necessary to have a fixture for each of them?

@astrojuanlu Could you let me know what do you think about the above questions?

Thanks!

[codecov]

Codecov Report

Merging #1255 (8c5a9c7) into main (0a0c005) will increase coverage by 0.19%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1255      +/-   ##
==========================================
+ Coverage   91.47%   91.67%   +0.19%     
==========================================
  Files          79       79              
  Lines        4143     4144       +1     
  Branches      362      364       +2     
==========================================
+ Hits         3790     3799       +9     
+ Misses        266      261       -5     
+ Partials       87       84       -3     

Impacted Files	Coverage Δ
src/poliastro/twobody/propagation.py	91.48% <ø> (-0.18%)	⬇️
src/poliastro/twobody/orbit.py	83.17% <100.00%> (-0.12%)	⬇️
src/poliastro/core/propagation/__init__.py	96.00% <0.00%> (+1.66%)	⬆️
src/poliastro/czml/extract_czml.py	92.85% <0.00%> (+3.17%)	⬆️

---

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 0a0c005...8c5a9c7. Read the comment docs.

[jorgepiloto]

The logic when building the ALL_PROPAGATORS macros is wrong! 😱

We are only including the propagators which are shared among the elliptic, parabolic and hyperbolic ones. Thank you for noticing this @Yash-10! 🚀

[jorgepiloto]

This could be fixed by doing:

ALL_PROPAGATORS = set(ELLIPTIC_PROPAGATORS + PARABOLIC_PROPAGATORS + HYPERBOLIC_PROPAGATORS)

[Yash-10]

Thanks @jorgepiloto for your answer! Also, while we were on that module, I noticed that the "Note" inside cowell's docstring mentioned poliastro.integrators. I thought it was removed some time back and I hence changed the docstring to mention scipy's solve_ivp.

[jorgepiloto]

Just some suggestions. After those, this is ready to be merged by my side 👍🏽

[astrojuanlu]

About having a global marker: yes, absolutely! I didn't know having a pytestmark would "apply marks at the module level" https://docs.pytest.org/en/6.2.x/example/markers.html#marking-whole-classes-or-modules

In #1253 I suggested to make expected_doc a fixture in case we wanted to reuse it for more than 1 test. If not, every test can have their own expected_doc and that's fine.

[astrojuanlu]

This looks good to me! Ready to merge, unless @Yash-10 wants to make some extra additions (like the global pytest marker)

[astrojuanlu]

(And pending on the comment @jorgepiloto left)

[Yash-10]

Thanks @astrojuanlu I added the global pytest marker.

[jorgepiloto]

All modifications looking good, approving this 👍🏽