hipspy / hips

Python library to handle HiPS
https://hips.readthedocs.io
12 stars 16 forks source link

Add plot for image in high level docs #68

Closed adl1995 closed 7 years ago

adl1995 commented 7 years ago

This PR is not ready to be merged. It just shows that the plot for jpg is not in RGB. So, there might be an issue in drawing.

cdeil commented 7 years ago

I think I found a way to prevent docs/plot.py from being collected (and failing) as a doctest. Add the line doctest_norecursedirs = docs/plot.py to setup.cfg at the end of the tool:pytest section.

My other suggestion would be that you plot the sky image with wcsaxes, so that it has coordinate axes.

So I think if you limit this PR to getting this image into the high-level docs, this PR could be finalised quickly.

adl1995 commented 7 years ago

@cdeil I made the changes. Let's see if the test case passes now...

cdeil commented 7 years ago

What do you get locally? Please share a screenshot here.

cdeil commented 7 years ago

Works for me locally, I get this: image

adl1995 commented 7 years ago

Here it is: plot

cdeil commented 7 years ago

This is looking good, this can go in soon.

I left two inline commments. My last comment would be to maybe change the name of the script a bit from plot.py to something more specific, at least plot_fits.py. Note that already tomorrow, we'll get a second script that plots an RGB image, and then a few more scripts in the docs in the future still.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 97.613% when pulling e7ab001f0976703238ce714c8831ea0d2bf9dc08 on adl1995:doc_update into 17dcb5052d0f1f9e55e5280ff87fa3951a90b859 on hipspy:master.

adl1995 commented 7 years ago

@cdeil I think this is ready to be merged now.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 97.613% when pulling e2541ae378982e9261dde09ed7b389a8d4b1b47b on adl1995:doc_update into 17dcb5052d0f1f9e55e5280ff87fa3951a90b859 on hipspy:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 97.613% when pulling e2541ae378982e9261dde09ed7b389a8d4b1b47b on adl1995:doc_update into 17dcb5052d0f1f9e55e5280ff87fa3951a90b859 on hipspy:master.

cdeil commented 7 years ago

I worked, image is here: https://hips.readthedocs.io/en/latest/getting_started.html

🎈 🎈 🎈

cdeil commented 7 years ago

Just for future reference, in case we discover some bug or change that example, the image at the moment looks like this:

image