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

Scatter Plot: multiple contexts and different Orange versions crash #3398

Closed
markotoplak opened this issue Nov 15, 2018 · 4 comments
Closed

Comments

@markotoplak
Copy link
Member

Orange version

master

Expected behavior

Files saved and working in earlier Orange versions won't crash future versions

Actual behavior

Migrate_context crashes under certain conditions. If widget's settings were saved with multiple contexts, only the active context gets migrated. If saved, some contexts are migrated and some are not.

Then, if migration code assumes that some things (like dict keys) are available, we get a crash.

Steps to reproduce the behavior
  1. On master, do File -> Scatter Plot. First send in iris, then housing. Save file.
  2. Open file in 3.16. Save it. Now you get the ows with Scatter Plot settings as below.
  3. Open the file again in master.

In the saved ows we have a mix of ScatterPlot's context version's 2 and 3 (with settings version 2). The ScatterPlot of master is unable to open that file because it assumes that the format is settings version 2. The problematic Scatter Plot setting are displayed below.

Additional info (worksheets, data, screenshots, ...)
{   '__version__': 2,
'auto_sample': True,
'auto_send_selection': True,
'context_settings': [   Context(attributes={'AGE': 2, 'B': 2, 'CHAS': 2, 'CRIM': 2, 'DIS': 2, 'INDUS': 2, 'LSTAT': 2, 'MEDV': 2, 'NOX': 2, 'PTRATIO': 2, 'RAD': 2, 'RM': 2, 'TAX': 2, 'ZN': 2},
                                metas={},
                                time=1542288286.4157782,
                                values= {   '__version__': 2,
                                           'attr_x': ('CRIM', 102),
                                           'attr_y': ('ZN', 102),
                                           'graph': {   'attr_color': (   'MEDV',
                                                                          102),
                                                        'attr_label': (   None,
                                                                          -2),
                                                        'attr_shape': (   None,
                                                                          -2),
                                                        'attr_size': (   None,
                                                                         -2)}}),
                        Context(attributes={'iris': 1, 'petal length': 2, 'petal width': 2, 'sepal length': 2, 'sepal width': 2},
                                metas={},
                                time=1542287688.3705914,
                                values= {   '__version__': 2,
                                           'attr_color': ('iris', 101),
                                           'attr_label': (None, -2),
                                           'attr_shape': (None, -2),
                                           'attr_size': (None, -2),
                                           'attr_x': ('sepal length', 102),
                                           'attr_y': ('sepal width', 102),
                                           'graph': {}})],
'controlAreaVisible': True,
'graph': {   'alpha_value': 128,
             'class_density': False,
             'jitter_continuous': False,
             'jitter_size': 0,
             'label_only_selected': False,
             'point_width': 10,
             'show_grid': False,
             'show_legend': True,
             'show_reg_line': False,
             'tooltip_shows_all': False},
'savedWidgetGeometry': b'\x01\xd9\xd0\xcb\x00\x02\x00\x00\x00\x00\x07\x7f'
                       b'\x00\x00\x00\x12\x00\x00\x0b\x9e\x00\x00\x046'
                       b'\x00\x00\x07\x80\x00\x00\x00\x13\x00\x00\x0b\x9d'
                       b'\x00\x00\x045\x00\x00\x00\x01\x00\x00\x00\x00'
                       b'\x07\x80',
'selection_group': None,
'toolbar_selection': 0}
@markotoplak
Copy link
Member Author

The old files are as they are and the only thing we can do there is to make the migrate code more robust.

But if not done already, we should prevent such context version mixing in saved files.

@lanzagar lanzagar added this to the 3.19 milestone Nov 27, 2018
@janezd janezd removed this from the 3.19 milestone Jun 3, 2019
@janezd janezd added the needs discussion Core developers need to discuss the issue label Jul 11, 2019
@janezd janezd self-assigned this Sep 6, 2019
@janezd
Copy link
Contributor

janezd commented Oct 10, 2019

I tried to replicate this. It didn't crash when I reopened on master and it could retrieve both contexts. I'm not sure why it works.

Partially related: should we prohibit opening new schemas on old versions of Orange? That is, check that the saved settings version is not higher than the settings version of the widget? If it is, what should happen? Do we have a suitable way to cancel the loading of schema so late?

@janezd janezd removed their assignment Oct 10, 2019
@lanzagar
Copy link
Contributor

A simple fallback could be to just reset/clear the widget settings that are newer than the version of Orange. I think prohibiting opening schemas at all is a bit of an overkill. Lots of widget settings changes are minor and a user could just reselect something in a widget and make the schema work again.

@janezd janezd removed the needs discussion Core developers need to discuss the issue label Nov 14, 2019
@janezd
Copy link
Contributor

janezd commented Dec 6, 2019

I tested this on current master: it migrates all settings.

When I went backwards to 3.16, it discarded all contexts?! As I understand, the problem occurs specifically when opening newer workflows on older version -- they add new contexts and incorrectly save newer contexts with older numbers? If so, I think the problem is that we allow opening newer workflows with older versions of Orange. This cannot not crash. I would still close this issue and let biolab/orange-canvas-core#66 take care of the problem.

@janezd janezd closed this as completed Dec 6, 2019
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

No branches or pull requests

3 participants