biolab / orange3

šŸŠ :bar_chart: :bulb: Orange: Interactive data analysis
https://orangedatamining.com
Other
4.85k stars 1.01k forks source link

Autosave checkbox status in Save Data widget not saved in ows file if reopened #4932

Closed ingebert closed 4 years ago

ingebert commented 4 years ago

Hi, I use Orange 3.26.0 on Win 10 and used the standard 64 bit exe installer. (thank you so much for your awesome software and esecially the interactive t-SNE viewer widget!) I want to automatically safe a csv file every time the Orange is startet via the corresonding .ows file. Unfortunately, the status of the checkbox "Autosave when resceiving new data" of the "Save Data" widget is not stored/saved in the ows file. If I check the box, safe the .ows file and reopen the file, the checkbox is unchecked again: image I used this file: Interactive_Visualizations_ows.zip It seems like, this information is not passed to the ows file.

PrimozGodec commented 4 years ago

This property is actually saved in .ows but changed in the widget initialization when opening the saved workflow. It happens here: https://github.com/biolab/orange3/blob/6193d329e7e71698b0c19c5d57b64ca5682f1ed4/Orange/widgets/utils/save/owsavebase.py#L130 I think it is made so to prevent overriding existing file when opening the saved workflow. Am I right @janezd?

ingebert commented 4 years ago

OK thanks a lot for the information!
As this seems to be an intended behaviour, I will close this issue. For the record: I changed line 130 in my owsavebase.py file to get the desired functionality : "#self.auto_save = True" In the original code it looks like, the "self.auto_save = False" is not set, if the path is relative. Unfortunately I can only set an absolute path (even if the file is saved in the same folder as the ows file, see screenshot from help doc). image I donĀ“t know if this is wanted or not, at least itĀ“s not what the help documentation says.

PrimozGodec commented 4 years ago

Unfortunately I can only set an absolute path (even if the file is saved in the same folder as the ows file, see screenshot from help doc).

self._absolute_path is always absolute, but self.stored_path which is saved to .ows should be relative if the file is placed in a same dir or in a sub-dir. How did you set the absolute path?

I will reopen this issue to wait what @janezd tells about changing an autosave checkbox.

janezd commented 4 years ago

Yes, this is by design. One reason is to prevent overwriting an existing file and the other is to prevent writing a file to another person's disk by sending him an .ows, which seems like (a very innocent) security hole.

ajdapretnar commented 4 years ago

I would still like to keep in mind the users' desire to automate workflows. We should think about possible solutions and adjustments to current widgets to enable this.

ingebert commented 4 years ago

Thanks for your comments. I understand that this security hole is an issue and see why you implemented its prevention. @PrimozGodec I set the absolute path with the common Windows "Save As" File dialogue. There I can not set a relative path (or maybe IĀ“m too stupid ^^:) ) Yes @ajdapretnar this is exactly my point. Now I have to set this checkbox every time I open the file, which is a bit unhandy for a more or less automated workflow. An option where User can bypass this security issue consciously would be good.

irgolic commented 4 years ago

I'm with @ajdapretnar on preserving automation and portability of workflows.

Regarding the first reason (to prevent overwriting an existing file), this seems to only be a problem if the user is playing around with the workflow, making changes they do not wish to save. If one wishes to keep a definitive version of a saved file, they should use version control.

Regarding the second reason (preventing writing a file), opening and running a workflow to generate files could be the intended outcome. To patch the security hole, maybe we could constrain files to be written only in the directory of the .ows and its subdirectories? Also, this got me thinking, how should we handle OWPythonScript as a security hole, especially if it implements IPython?