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

[FIX] Prevent crash when saving in unsupported format #5560

Merged
merged 2 commits into from
Sep 10, 2021

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Aug 23, 2021

Issue

Fixes #5512.

The situtation can occur if workflow is saved with Orange that has add-ons with additional file formats and then opened in Orange without that add-on, or if support for some format is dropped (as was the case with old Excel, which caused #5512).

Description of changes

Save widget base

Widgets for saving are derived from OWSaveBase, which fails in property writer. This PR changes it to return None for filters that are not in the list. The only place where writer is used is do_save, which in this case now indicates an error.

The error is cleared at the only place that changes self.filter: if the user selects a new file name, the filter cannot be invalid.

Derived widgets

Derived widgets need changes only if they use self.writer, in which case they must check that it is not None. Among the three core widgets, only Save does, and this PR adapts it accordingly. Save Model and Save Distances shouldn't need any changes.

The consequence of not checking for self.writer is None is a crash -- like previously, just a bit later (when using the property instead of when obtaining it).

Includes

  • Code changes
  • Tests
  • Documentation N/A

@codecov
Copy link

codecov bot commented Aug 23, 2021

Codecov Report

Merging #5560 (e2ecd97) into master (7de3419) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5560   +/-   ##
=======================================
  Coverage   85.91%   85.92%           
=======================================
  Files         313      313           
  Lines       65349    65365   +16     
=======================================
+ Hits        56143    56163   +20     
+ Misses       9206     9202    -4     

@janezd
Copy link
Contributor Author

janezd commented Aug 27, 2021

Check widgets for reading.

@janezd
Copy link
Contributor Author

janezd commented Aug 27, 2021

I checked: the File widget does not crash and even has a test for it. Load Model and Distance File have only a single extension and no registry. Unlike the widgets for saving, widgets for reading do not have a common base, so there's nothing to fix.

This can be reviewed as is. Volunteers welcome.

Copy link
Contributor

@VesnaT VesnaT left a comment

Choose a reason for hiding this comment

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

Apart from that typo, I'd merge.

@@ -43,6 +43,7 @@ class Information(widget.OWWidget.Information):

class Error(widget.OWWidget.Error):
no_file_name = widget.Msg("File name is not set.")
unsupported_format = widget.Msg("File fornat is unsupported.\n{}")
Copy link
Contributor

Choose a reason for hiding this comment

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

fornat -> format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thamks! Fixed.

@VesnaT VesnaT merged commit f1ecc3c into biolab:master Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Widget Save data not working propperly
2 participants