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

Add warning when empty segmentation is passed with omit_empty_frames #181

Merged
merged 2 commits into from
Nov 9, 2022

Conversation

CPBridge
Copy link
Collaborator

@CPBridge CPBridge commented Jul 6, 2022

Addresses #180

When the omit_empty_frames option is used for a Segmentation and an empty segmentation mask is passed (i.e. a mask with all zeros), the constructor will issue a UserWarning and ignore the omit_empty_frames option.

Add tests

@CPBridge CPBridge added the bug Something isn't working label Jul 6, 2022
@CPBridge CPBridge requested a review from hackermd July 6, 2022 23:58
Copy link
Collaborator

@hackermd hackermd left a comment

Choose a reason for hiding this comment

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

Thanks @CPBridge! I am wondering whether it should be a user warning or a warning level log message. I would probably favor the latter, because it may be nice to know for which instance the warning is issued, because the fact that all frames are empty could be due to a problem in the application logic/code. What do you think?

@CPBridge
Copy link
Collaborator Author

CPBridge commented Jul 7, 2022

I am wondering whether it should be a user warning or a warning level log message. I would probably favor the latter, because it may be nice to know for which instance the warning is issued, because the fact that all frames are empty could be due to a problem in the application logic/code

I have changed to a logger warning for now. But we should probably wait for a consensus on #180 before merging

@CPBridge CPBridge merged commit 4b13338 into master Nov 9, 2022
@CPBridge CPBridge deleted the empty_seg_error_message branch November 9, 2022 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants