Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: report errors, retry in saving camera image #21

Merged
merged 8 commits into from
Dec 5, 2024
93 changes: 69 additions & 24 deletions camviewer_ui_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
QClipboard,
QPixmap,
QDrag,
QImageWriter,
)
from PyQt5.QtCore import (
QTimer,
Expand Down Expand Up @@ -185,6 +186,7 @@ class GraphicUserInterface(QMainWindow):
param1Update = pyqtSignal()
param2Update = pyqtSignal()
timeoutExpiry = pyqtSignal()
retry_save_image = pyqtSignal()

def __init__(
self,
Expand Down Expand Up @@ -522,6 +524,7 @@ def __init__(
self.setOrientation(param.ORIENT0) # default to use unrotated

self.ui.FileSave.triggered.connect(self.onfileSave)
self.retry_save_image.connect(self.onfileSave)

self.imageUpdate.connect(self.onImageUpdate)
self.miscUpdate.connect(self.onMiscUpdate)
Expand Down Expand Up @@ -1151,33 +1154,75 @@ def shutdown(self):

def onfileSave(self):
try:
fileName = QFileDialog.getSaveFileName(
self,
"Save Image...",
self.cwd,
"Images (*.npy *.jpg *.png *.bmp *.pgm *.tif)",
filename = QFileDialog.getSaveFileName(
KaushikMalapati marked this conversation as resolved.
Show resolved Hide resolved
parent=self,
caption="Save Image...",
directory=os.path.expanduser("~"),
filter="Images (*.npy *.jpg *.png *.bmp *.pgm *.tif)",
)[0]
if fileName == "":
raise Exception("No File Name Specified")
if fileName.lower().endswith(".npy"):
np.save(fileName, self.image)
QMessageBox.information(
self,
"File Save Succeeded",
"Image has been saved as a numpy file: %s" % (fileName),
)
print("Saved to a numpy file %s" % (fileName))
if filename == "":
KaushikMalapati marked this conversation as resolved.
Show resolved Hide resolved
# The only way to get here is by clicking "X" or "Cancel"
return
file_ext = os.path.splitext(filename)[1]
if file_ext == ".npy":
# This either works or raises an OSError such as PermissionError
np.save(filename, self.image)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double checking, but can this fail silently? In the cases you check for if QImage.save fails (no write permission, file already exists, other), can this fail by throwing an exception which would not result in a retry? Also looking at https://github.com/numpy/numpy/blob/v2.1.0/numpy/lib/_npyio_impl.py#L507-L588
I think that this could override an existing file with the same name which shouldn't be possible in case someone tries to use the same filename for different images and ends up overwriting all but the last one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that I need to be more careful here. The exception being thrown would be "safe" but wouldn't result in a retry, giving inconsistent behavior.

I'll double-check about re-using the same name, but my fuzzy memory is that the file dialog gives you an "are you sure?" sort of error, so if we reach np.save the user has already confirmed that they want to replace the file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved by refactoring to make specifically OSError call the same method that we reach via the QImage.save checks.
Confirmed that the file select dialog warns the user sufficiently if they pick an already existing filename.

return self.show_file_success(filename=filename, file_ext=file_ext)
save_ok = self.ui.display_image.image.save(
filename, format=None, quality=-1
)
if save_ok:
# QImage.save returned True, so the save succeeded.
return self.show_file_success(filename=filename, file_ext=file_ext)
# QImage.save returned False, so the save failed.
# Check for obvious errors, then retry the save
# No write permissions?
# If the file exists, we need to check the filename. Otherwise, we check the dirname.
if os.path.exists(filename):
can_write = os.access(path=filename, mode=os.W_OK)
else:
self.ui.display_image.image.save(fileName, format=None, quality=-1)
QMessageBox.information(
self,
"File Save Succeeded",
"Image has been saved to an image file: %s" % (fileName),
directory = os.path.dirname(filename)
can_write = os.access(path=directory, mode=os.W_OK)
if not can_write:
return self.warn_and_retry_save(
message=f"No permissions to write {filename}! Please pick a different location."
)
print("Saved to an image file %s" % (fileName))
except Exception as e:
print("fileSave failed:", e)
QMessageBox.warning(self, "File Save Failed", str(e))
# Invalid image type?
image_types = [".npy"] + [
"." + qba.data().decode("utf-8")
for qba in QImageWriter.supportedImageFormats()
]
if file_ext not in image_types:
return self.warn_and_retry_save(
message=f"Invalid image type {file_ext}! Please pick from the list {image_types}."
)
# I guess we have no idea what went wrong
return self.warn_and_retry_save(
message="Unknown failure! Please try again."
)
except OSError as exc:
self.warn_and_retry_save(message=str(exc))
except Exception as exc:
print("fileSave failed:", exc)
QMessageBox.warning(
self, "File Save Failed", f"Internal error, cancelling save: {exc}"
)

def show_file_success(self, filename: str, file_ext: str):
QMessageBox.information(
self,
"File Save Succeeded",
f"Image has been saved as a {file_ext} file: {filename}",
)
print(f"Saved to a {file_ext} file: {filename}")

def warn_and_retry_save(self, message: str):
QMessageBox.warning(
self,
"File Save Failed",
message,
)
self.retry_save_image.emit()

def onOrientationSelect(self, index):
self.setOrientation(param.idx2orient[index], fromCombo=True)
Expand Down