[ENH] OWMDS, OWLinearProjection: save selection in workflow

[kernc]

Issue

Fixes #2182

Description of changes

Includes

• Code changes
• Tests
• Documentation

[codecov-io]

Codecov Report

Merging #2301 into master will increase coverage by 0.01%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master    #2301      +/-   ##
==========================================
+ Coverage   74.53%   74.55%   +0.01%     
==========================================
  Files         321      321              
  Lines       56134    56140       +6     
==========================================
+ Hits        41838    41853      +15     
+ Misses      14296    14287       -9

[astaric]

Selections are not really context settings as they depend not only on the columns in the dataset but on the number of rows as well. Have you considered using schema only settings?

[kernc]

The problem I have with schema_only settings is that there's additional boilerplate associated with using them:

    def closeContext(self):
        if self.current_context is not None:
            self.selection = None
        super().closeContext()

Short, but very intricate.

[janezd]

File -> Sample -> MDS. Select all points in MDS. Decrease the sample size in Sample. MDS crashes due to selection indices out of range. Linear projection probably as well.

A saved context setting is reused when "useful", e.g. if the setting includes a variable, the variable has to appear in the domain for the context to be useful. In this sense, the context here is not the domain but the sample size (e.g. the saved selection is "useful" if there is enough data for these indices). However, this context would be too vaguely defined, so checking the domain (DomainContextHandler) is OK.

One quick solution would be to discard the selection (after openContext) if max(selection_indices) >= len(data). A slightly better solution is to have DomainAndDataSizeContextHandler, but it can still lead to weird selections. The safest solution would be DataHashContextHandler, where the has is computed over X, Y and metas. The latter would also make sense for some other widgets.

[janezd]

Also, add tests that will check for this.

[kernc]

Right. How is this?

[astaric]

Actually, they are.

As non-context-aware widgets only load settings when a new instance of the widget is put on the canvas, this is achieved by not updating default values. See https://github.com/biolab/orange3/blob/master/Orange/widgets/settings.py#L508

[kernc]

But what if the same instance of widget but passed different data, as described here?

[astaric]

With this change, https://github.com/biolab/orange3/blob/master/Orange/widgets/visualize/owscatterplot.py#L581 is now redundant.

As the feature was implemented in a simple way, the suggestion is no longer relevant.

[astaric]

For settings, primitive types > numpy, as they can be serialized to json, resulting in human readable settings.

[astaric]

This looks good to me, rebase when possible so we can merge.

[lanzagar]

What is this change doing in a commit "save selection in workflow" ?!?

Did you change this while testing and accidentally committed it?
Or decided to change some defaults and make it less obvious by not mentioning it at all and/or putting it in its own commit.