AndrewAnnex / SpiceyPy

SpiceyPy: a Pythonic Wrapper for the SPICE Toolkit.
MIT License
384 stars 84 forks source link

Context manager for SPICE kernels #458

Closed alfonsoSR closed 1 year ago

alfonsoSR commented 1 year ago

Modifications:

Solves #454

codecov[bot] commented 1 year ago

Codecov Report

Merging #458 (ee414a1) into main (63330ea) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #458   +/-   ##
=======================================
  Coverage   99.87%   99.87%           
=======================================
  Files          12       13    +1     
  Lines       15578    15633   +55     
=======================================
+ Hits        15559    15614   +55     
  Misses         19       19           
Impacted Files Coverage Δ
src/spiceypy/spiceypy.py 99.71% <100.00%> (+<0.01%) :arrow_up:
src/spiceypy/tests/test_context_manager.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

AndrewAnnex commented 1 year ago

okay this is looking pretty good now, just a small edit suggestion to capture some of our discussion on the caveats in a "warning" rst field near the top of the new documentation page. Once the extension is changed to .rst I can test the doc builds on readthedocs to ensure they render.

alfonsoSR commented 1 year ago

I requested the review unintentionally😅

AndrewAnnex commented 1 year ago

thanks @alfonsoSR! it's merged.

alfonsoSR commented 1 year ago

Great! This was my first time contributing to an open source project, and it feels pretty good! I also learnt a couple of interesting things along the way, so thank you very much for the time and the feedback! @AndrewAnnex