High-level function: make_sky_image

[adl1995]

Added a function make_sky_image and passed it by adding the @pytest.mark.xfail decorator. The high level functionality for this is yet to be discussed.

[coveralls]

Coverage increased (+0.02%) to 96.875% when pulling 2e1a9dc5bc68ecdff9a57b47c790bfb5354c281a on adl1995:tiles.draw into 86284662570e94c0610729c2bf9178e19b74ee09 on hipspy:master.

[cdeil]

@adl1995 - Please make a first draft of this function, i.e. actually write inputs, code and return something, and write the docstring and test.

There's some info here: https://github.com/hipspy/hips/blob/master/notes/API-proposal.md#make-a-wcs-image and we also discussed it a few weeks ago here: https://docs.google.com/document/d/1D3bEJuSH4d99u9dH_hGur3jbbPNapRI2hduYHCH9NKk/edit#heading=h.paw124lueu5w (the make_sky_image(geometry, hips_survey)).

By now, we've introduced the WCSGeometry class, that should be the "geometry" input mentioned.

If you want, you could also start to add classes for the missing functionality here. I mean just put the classes with a docstring and the methods you need and put "raise NotImplementedError" in the methods.

So what we do here in this PR is to actually write down the API we want as actual Python functions / classes, they just aren't implemented yet. Then we'll merge, and you'll implement the missing pieces in small PRs one at a time next week and I think by the end of next week, we'll have something we can call v0.1 and release. :-)

[cdeil]

I see you've renamed hips/draw/simple.py → hips/draw/tile.py here. I'd suggest you don't introduce hips/draw/tile.py and keep as-is. All of our drawing / functionality will be tile-related. The idea with simple.py was that this is where you implement https://hips.readthedocs.io/en/latest/drawing_algo.html#naive-algorithm and then https://hips.readthedocs.io/en/latest/drawing_algo.html#precise-algorithm will go into some other file, like precise.py.

We might also need a high_level.py or something like that with a uniform interface into the different drawing methods. I'd be OK if you add high_level.py now if you like, and only have the simple tile drawing stuff in simple.py.

[coveralls]

Coverage decreased (-1.7%) to 78.715% when pulling 54a19c33c976aa79d5685f5a3e4335967a851ace on adl1995:tiles.draw into fe75a714870e925ade8f2b8387fb3c01a2b2aa6c on hipspy:master.

[coveralls]

Coverage decreased (-1.7%) to 78.715% when pulling 54a19c33c976aa79d5685f5a3e4335967a851ace on adl1995:tiles.draw into fe75a714870e925ade8f2b8387fb3c01a2b2aa6c on hipspy:master.

[coveralls]

Coverage decreased (-1.3%) to 79.116% when pulling 8add0250a612c22b9a5ecb5dbd4ce9e4a9369f89 on adl1995:tiles.draw into 208a73e9c26e4def414c1492f2a02fe8961ef82c on hipspy:master.

[adl1995]

@cdeil - I just made a commit adding the missing code. I made an assert on [0, 0]. Although it is passing, I don't think the result is okay. I tried displaying the resulting image through matplotlib and all the image is black except for a small portion at the top. This leads me to think that the tiles were outside the sky image. Could you please take a look?

[adl1995]

Also, the resulting image of test_draw_sky_image was all zeros, so I marked it with pytest.mark.xfail.

[adl1995]

@cdeil - I moved the _draw_sky_image_one_tile function to DrawSkyImageOneTile class. Also, I verified the output through matplotlib and the result looks okay.

[adl1995]

@cdeil - I just made some commits. Please check this (https://github.com/hipspy/hips/pull/30/files#diff-166822ec0a0c3100c6f9ffdd92c4b76eR19) line. I don't think the mypy notation Generator[HipsTile, Any, Any] is correct in this case, but I could not find another solution at the moment.

[cdeil]

At the end of the tests there's these warnings: https://travis-ci.org/hipspy/hips/jobs/248660210#L2173

/private/var/folders/my/m6ynh3bn6tq06h7xr3js0z7r0000gn/T/hips-test-0qyifajc/lib.macosx-10.9-x86_64-3.6/hips/draw/tests/test_simple.py:24: ResourceWarning: unclosed file <_io.FileIO name='/Users/travis/hips-extra/datasets/samples/DSS2Red/Norder3/Dir0/Npix451.fits' mode='rb' closefd=True>

The reason is that your fits.open calls leave open file handles. Please change the FITS reading in HipsTile.read to this pattern, that should (I hope) resolve the issue, i.e. close the file handles at the end of the with block, before exiting the read method call:

with fits.open(str(path) as hdu_list:
        data = hdu_list[0].data
        header = hdu_list[0].header
return cls(meta, data, header)

[cdeil]

Please make sure all tests are passing (or ask me for help if you don't know how to resolve some fails). See https://travis-ci.org/hipspy/hips/jobs/248660210#L2135

[cdeil]

Do you know what's up with this warning? https://travis-ci.org/hipspy/hips/jobs/248660210#L2161

/Users/travis/miniconda/envs/test/lib/python3.6/site-packages/astropy/io/fits/file.py:336: AstropyUserWarning: File may have been truncated: actual file length (527168) is smaller than the expected size (529920)
    self.size, pos), AstropyUserWarning)

Can you produce a few lines with a standalone test case that results in the same issue so that we can understand / debug / fix the issue?

[adl1995]

@cdeil I made another series of commits. Currently waiting to Travis CI build to complete.

[adl1995]

@cdeil I'm not sure why this example is failing to execute (https://travis-ci.org/hipspy/hips/jobs/248711430#L1182). I have defined hips_survey in the above line, but it still raises a NameError.

[coveralls]

Coverage increased (+6.8%) to 87.22% when pulling a4b6846ef107572e7b542114b731089346c3b1f5 on adl1995:tiles.draw into 32db7fbedfebbc1a77028c35f80b9fb45a483cc1 on hipspy:master.

[coveralls]

Coverage increased (+7.1%) to 87.54% when pulling 5fe3ed0aa5be6544a1b71b786f0014df626d1b21 on adl1995:tiles.draw into 32db7fbedfebbc1a77028c35f80b9fb45a483cc1 on hipspy:master.

[coveralls]

Coverage increased (+7.1%) to 87.54% when pulling 5fe3ed0aa5be6544a1b71b786f0014df626d1b21 on adl1995:tiles.draw into 32db7fbedfebbc1a77028c35f80b9fb45a483cc1 on hipspy:master.

[cdeil]

Travis-ci is all green now: https://travis-ci.org/hipspy/hips/builds/248735077?utm_source=github_status&utm_medium=notification

@adl1995 - Merge now? Or do you want to continue today and try to fix the computation of the list of tiles needed here to take into account the Hips and sky image coordinate frame correctly?

[cdeil]

After some more debugging and pair-coding with @adl1995 we have a working make_sky_image, see the attached screenshot of the image resulting from the high-level docs example.

Well, it's kinda working, you can see that the summing of the tiles creates bright line artefacts at the edges, and that tiles are missing because they query_disk doesn't work properly.

Let's discuss those and other things in separate follow-up issues. I'm merging this now.

@adl1995 - Good job! And have a nice weekend!

[coveralls]

Coverage increased (+6.8%) to 87.267% when pulling b739539ddea7423bec09fec5d256ead87e8f9e15 on adl1995:tiles.draw into 32db7fbedfebbc1a77028c35f80b9fb45a483cc1 on hipspy:master.

[tboch]

Well done, that's a valuable progress!