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] Data Sampler: Fix crash when requesting an empty sample #6208

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Nov 18, 2022

Issue

Fixes #6207.

Description of changes

Widget already treated a 100 % sample as special case. The same code is now also used for empty sample.

Related change: when setting the proportion, widget allowed to set 0 %, but when setting the number of instances, it enforced a minimum of 1. Now it also allows 0.

Includes
  • Code changes
  • Tests

@codecov
Copy link

codecov bot commented Nov 18, 2022

Codecov Report

Merging #6208 (b2b6b77) into master (0d3e3ee) will increase coverage by 0.33%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6208      +/-   ##
==========================================
+ Coverage   86.66%   86.99%   +0.33%     
==========================================
  Files         316      316              
  Lines       68021    69250    +1229     
==========================================
+ Hits        58948    60245    +1297     
+ Misses       9073     9005      -68     

@VesnaT
Copy link
Contributor

VesnaT commented Nov 25, 2022

A Data Table widget does not work with this change:

This is actually an old issue, since the Remaining Data could have had length 0.

image

Traceback (most recent call last):
  File "/Users/vesna/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 1024, in __process_next
    if self.__process_next_helper(use_max_active=True):
  File "/Users/vesna/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 1062, in __process_next_helper
    self.process_node(selected_node)
  File "/Users/vesna/orange-canvas-core/orangecanvas/scheme/signalmanager.py", line 690, in process_node
    self.send_to_node(node, signals_in)
  File "/Users/vesna/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 806, in send_to_node
    self.process_signals_for_widget(node, widget, signals)
  File "/Users/vesna/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 820, in process_signals_for_widget
    process_signals_for_widget(widget, signals, workflow)
  File "/Users/vesna/miniconda3/lib/python3.8/functools.py", line 875, in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
  File "/Users/vesna/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 921, in process_signals_for_widget
    process_signal_input(input_meta, widget, signal, workflow)
  File "/Users/vesna/miniconda3/lib/python3.8/functools.py", line 875, in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
  File "/Users/vesna/orange-widget-base/orangewidget/workflow/widgetsscheme.py", line 884, in process_signal_input_default
    notify_input_helper(
  File "/Users/vesna/miniconda3/lib/python3.8/functools.py", line 875, in wrapper
    return dispatch(args[0].__class__)(*args, **kw)
  File "/Users/vesna/orange-widget-base/orangewidget/utils/signals.py", line 801, in set_multi_input_helper
    handler(*args)
  File "/Users/vesna/orange3/Orange/widgets/data/owtable.py", line 292, in set_dataset
    slot = self._inputs[index]
IndexError: list index out of range
-------------------------------------------

@VesnaT VesnaT mentioned this pull request Nov 25, 2022
3 tasks
@janezd
Copy link
Contributor Author

janezd commented Nov 25, 2022

You can reproduce this on master, by selecting all data and connecting Remaining Data instead of Data Sample.

This PR just makes the bug (which is unrelated to this widget) easier to reproduce. :)

@janezd janezd closed this in #6221 Nov 25, 2022
@janezd janezd reopened this Nov 25, 2022
@VesnaT VesnaT merged commit 3423a8e into biolab:master Dec 1, 2022
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.

Data Sampler: Fixed proportion with 0% crash
2 participants