poliastro / czml3

Python 3 library to write CZML
https://pypi.org/project/czml3/
MIT License
38 stars 30 forks source link

Added support for CZML rectangles #58

Closed idanmiara closed 4 years ago

idanmiara commented 4 years ago

core.py - added rectangle attribute to the CZML Packet class properties.py - added CartographicRectangle, RectangleCoordinates CZML object classes

codecov-io commented 4 years ago

Codecov Report

Merging #58 into master will decrease coverage by 0.14%. The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
- Coverage   98.96%   98.82%   -0.15%     
==========================================
  Files          12       12              
  Lines         582      596      +14     
==========================================
+ Hits          576      589      +13     
- Misses          6        7       +1
Impacted Files Coverage Δ
src/czml3/core.py 100% <100%> (ø) :arrow_up:
src/czml3/properties.py 99.62% <92.3%> (-0.38%) :arrow_down:

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 f53fbde...ed37120. Read the comment docs.

idanmiara commented 4 years ago

Hi, I've reformatted properties.py with black and forced push, but I then I couldn't update the pull request, so I've closed it and opened this one. hope it's OK now.

astrojuanlu commented 4 years ago

No problem @imiara! Now quality is good and nothing broke, but we should add some tests so the coverage does not decrease :) Do you feel confident to use some of the existing ones as inspiration?

astrojuanlu commented 4 years ago

Thanks @idanmiara , this is almost there! isort is complaining about the order of the imports (yes, we're that picky). Just apply $ tox -e reformat (you need pip install tox first) and commit the results.

astrojuanlu commented 4 years ago

For example https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile

idanmiara commented 4 years ago

Hi, I don't know MyPy, but these function do exist in my environment. Maybe it doesn't like that they are underscored prefixed. Anyway I did find that there is a built in function that I can use instead, so no problem. tempfile.NamedTemporaryFile creates a temp file, I just needed a temp filename - tempfile.mktemp() does that. Could you tell me where do you find these reports or how do I run all these tests myself before pushing it? I've tried to look into the codecov report but I couldn't find what's going on there.

astrojuanlu commented 4 years ago

Hi @idanmiara - to see these checks, you can click here:

Screenshot_2020-02-20 Added support for CZML rectangles by idanmiara · Pull Request #58 · poliastro czml3

And see the details of every step.

To check if the quality step will pass you don't need mypy either, doing tox -e quality will run isort, mypy, black and flake8. Also, if you do tox -e test, all the tests will be run.

idanmiara commented 4 years ago

@Juanlu001 Thank you very much for spending the time with this code review and for merging it! When do you plan to make new releases and publish to PyPI and to Conda ? Do you do it regularly ?

astrojuanlu commented 4 years ago

I made a couple of fixes: #59 and #60 (so it ended up being called Rectangle like the original at https://github.com/AnalyticalGraphicsInc/czml-writer/wiki/Rectangle).

For the moment I don't do releases regularly, so now that I'm at it I'll make a new one with these changes :) Stay tuned!

astrojuanlu commented 4 years ago

czml3 0.5.3 available with Rectangle :tada: