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
Validate the data in StartupDialog and handle mismatched segmentation labels #125
Validate the data in StartupDialog and handle mismatched segmentation labels #125
Conversation
Nice, looks good on first sight. Will review as soon as I have my laptop. First small comment: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Please have a look at the comments and then feel free to squash and merge.
labelCloud/io/labels/config.py
Outdated
@@ -30,6 +30,22 @@ def to_dict(self) -> dict: | |||
} | |||
|
|||
|
|||
class ZeroLabelException(Exception): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move them into a new file .../labels/exceptions.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done here e4ce835
for c in self.classes: | ||
if c.id == self.default: | ||
return c.name | ||
raise DefaultIdMismatchException( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Didn't consider this case so far.
f"Segmentation labels {unique_label_ids} of `{self.path}` don't match with the label config {unique_class_ids}." | ||
) | ||
labels_to_replace = unique_label_ids.difference(unique_class_ids) | ||
msg.setInformativeText( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is happening in the case the user clicks on "Cancel"? ... it keeps the old (unknown) label?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It is done this way in case the user doesn't want to change their label files. However, if the user doesn't agree to replace, it will prompt every time it's checked.
labelCloud/model/point_cloud.py
Outdated
msg.setInformativeText( | ||
f""" | ||
Do you want to overwrite | ||
the mismatch labels {labels_to_replace} with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe "undefined labels" instead of "mismatch labels"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done here 84558e1
labelCloud/view/gui.py
Outdated
@@ -20,6 +19,8 @@ | |||
QMessageBox, | |||
) | |||
|
|||
from labelCloud.view.startup_dialog import StartupDialog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use relative imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done here 1f8fa59
This PR addresses:
Screenshots of the error handling in the StartupDialog
No label defined
default id in the _classes.json doesn't present in the dialog
label ids are not unique
label name is empty
Unhandled error
Screenshots of the message box when the segmentation labels don't match with LabelConfig