search

AstroHuntsman / gunagala

4 stars 9 forks source link

Crval not set as per Issue #17 #25

Closed lspitler closed 5 years ago

lspitler commented 5 years ago

As per https://github.com/AstroHuntsman/gunagala/issues/17 this is a simple fix to ensure CRVAL is always set at beginning of function call.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.7%) to 87.751% when pulling 583059871b385a9dbb10e6d73b8d6f40a054b738 on lspitler:crval_notset#17 into 7b6fcbfe892341cb4ec7a672ec4f04c064d2e866 on AstroHuntsman:image_sim.

lspitler commented 5 years ago

OK @AnthonyHorton I can try! I'm going to move self.wcs.wcs.crval = [centre.icrs.ra.value, centre.icrs.dec.value] out of get_pixel_coords completely....

lspitler commented 5 years ago

OK @AnthonyHorton please take a look at the new method.

lspitler commented 5 years ago

OK @AnthonyHorton try the current version. CRVAL defaults to [0., 0.] which I guess is technically valid, so I set it to [None, None] in __init__ of Imager to ensure it is set.

lspitler commented 5 years ago

@AnthonyHorton OK will try... why did Travis fail? I can't see the error - do I need a login? Locally, 4 tests fail in test_imager.py, but they did before I started coding.

AnthonyHorton commented 5 years ago

@lee The test fail is in the docs build, you should be able to see it here: https://travis-ci.org/AstroHuntsman/gunagala/jobs/432334464

For some reason the Sphinx auto docs fails when you import something into a module namespace, unless you specifically exclude it from the docs. In this case it's the new import of InvalidTransformError from astropy.wcs that's tripped it up. You'll need to edit gunagala/docs/index.rst and add a :skip: InvalidTransformError line in the .. automodapi:: gunagala.imager. You'll see there's a bunch of skip directives there I've already had to add. There's probably a way to avoid this, but I haven't found it yet.

Everything else is passing in Travis CI so I'm a bit concerned you're seeing local test failures. Which ones are failing, and how?

lspitler commented 5 years ago

@AnthonyHorton thanks for the tip about Sphinx. Just pushed fix for it and a test function. The local test failures are actually xFails, so never mind them. Welcome feedback on the test.

lspitler commented 5 years ago

@AnthonyHorton can you take a quick look to see if what I did with custom arg parsers is what you had in mind? Should be a quick one and will help @bazkiaei with mock galaxies.

lspitler commented 5 years ago

Ok @AnthonyHorton is a bottleneck for @bazkiaei 's code so I've merged & closed this as its been 6 days and I'm sure you have more important things to do than this tiny, tiny issue. Please re-open if there any problems and I'll fix via a new branch.