hipspy / hips

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

Add overwrite option in write_image function for FITS files #129

Closed adl1995 closed 6 years ago

adl1995 commented 6 years ago

Following issue #127, this PR adds the overwrite parameter to write_image function, which defaults to False. For now, this parameter only works with FITS files, I'm not sure what's the default behavior of PIL for JPG and PNG files.

@cdeil - Should I include a test case for this by writing a same file twice?

cdeil commented 6 years ago

@adl1995 - Thanks!

Well ideally, this option would work the same for FITS, PNG and JPEG. I guess if Pillow doesn't offer the equivalent you have to do something like:

from pathlib import Path
if Path(filename).exits():
    raise ...

Concerning test: yes, would be nice and shouldn't be hard to write (using with pytest.raises), but I'd also merge without since the code is simple.

adl1995 commented 6 years ago

@cdeil - Please see the updated code.

For FITS files, astropy.io.fits.writeto raises the OSError exception by default. However, for JPG and PNG files I have manually raised the FileExistsError exception, as it seems more appropriate.

Is this fine, or should all formats raise the same exception for consistency?

cdeil commented 6 years ago

I think if you move the code that checks and raises FileExistsError at the top, it will be nicer for the user, and already for you in the test, because it behaves the same.

Note that you should create an FileExistsError instance with the filename mentioned in the raise statement, as explained here: https://stackoverflow.com/questions/36077266/how-do-i-raise-a-filenotfounderror-properly

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.01%) to 96.286% when pulling 21cfd43eea4d0934074597fcd54e663af46ae27d on adl1995:io/overwrite into ead742fe422fd0d8b3f2227aba5be0685d603fd0 on hipspy:master.

adl1995 commented 6 years ago

This seems better. The FileExistsError is now raised for all file formats (FITS, JPG, PNG).

cdeil commented 6 years ago

Looks good. Thanks!