Fixes to ComparisonViewer for release version of ImageWidget #108
Conversation
…changed in recent astropy?), now checks len()>0 to see if it should be processed.
…wer and fixed astrowidget ImageWidget missing feature (no overwrite on save).
| @@ -707,11 +717,23 @@ def save_tess_files(self, button=None): | |||
| """ | |||
| if self._field_name.value: | |||
| self.tess_field_view() | |||
| self.iw.save(self._field_name.value, overwrite=True) | |||
| # Remove output file if it exists | |||
| if os.path.exists(self._field_name.value) and self.overwrite_outputs: | |||
There was a problem hiding this comment.
It would be nice to switch this to pathlib instead. I can do that later in a separate PR if you prefer,
There was a problem hiding this comment.
I'll get this addressed shortly.
| self.iw.save(self._field_name.value, overwrite=True) | ||
| # Remove output file if it exists | ||
| if os.path.exists(self._field_name.value) and self.overwrite_outputs: | ||
| os.remove(self._field_name.value) |
There was a problem hiding this comment.
pathlib can also do the remove
There was a problem hiding this comment.
I'll get this addressed shortly.
stellarphot/io/tess.py
Outdated
There was a problem hiding this comment.
Make the GAIA aperture server hardcoded. Seemed to make sense.
There was a problem hiding this comment.
Removed redundant import, fixed if v_angle: to not evaluate as ambiguous in current astropy, added a parameter to allow overwriting of all output files by ComparisonViewer.
| @@ -707,11 +717,23 @@ def save_tess_files(self, button=None): | |||
| """ | |||
| if self._field_name.value: | |||
| self.tess_field_view() | |||
| self.iw.save(self._field_name.value, overwrite=True) | |||
| # Remove output file if it exists | |||
| if os.path.exists(self._field_name.value) and self.overwrite_outputs: | |||
There was a problem hiding this comment.
I'll get this addressed shortly.
| self.iw.save(self._field_name.value, overwrite=True) | ||
| # Remove output file if it exists | ||
| if os.path.exists(self._field_name.value) and self.overwrite_outputs: | ||
| os.remove(self._field_name.value) |
There was a problem hiding this comment.
I'll get this addressed shortly.
| Path(self._field_name.value).unlink() | ||
| try: | ||
| self.iw.save(self._field_name.value) | ||
| except: |
There was a problem hiding this comment.
Usually it is better to not use a bare except, I think, in case you accidentally capture an error message that is really a bug. I think we could get away with no try/except here, anticipating that if an error is raised it will probably be for something real, and should be reported to the user with the exception details/traceback.
There was a problem hiding this comment.
Oh crap, I forgot to capture the specific error. I ended up wrapping all the file creation calls that could fail due to attempting to overwrite a file without permission with a try..except OSError. That should be specific enough. I notice the errors still show up on the Jupyterlab log page, so may not really be much easier for users.
Apparently the release version of astrowidgets does NOT support overwrite as an option to saving an ImageWidget, I implemented the same functionality via the os package. I also implemented an implementation-wide parameter to allow overwriting on all output files.
Also fixed an issue where
if v_angle:was triggering an exception asambiguous(I suspect this was a new behavior added in a recent version of astropy), instead of having v_angle evaluate as true is not empty. I replaced it with a check ofif len(v_angle):which evaluates the desired way.I also went into the code for retrieving GAIA stars and made the server a constant instead of a parameter to be passed in (it is still a public attribute and can be seen).