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] Preprocess - Store settings for unselected preprocessors #1042

Closed

Conversation

PrimozGodec
Copy link
Collaborator

@PrimozGodec PrimozGodec commented Mar 13, 2024

While working on #963, I found the following issue.

Issue

When a preprocessor (for example, one of the normalizers) is not selected, its settings are not stored in widget settings or workflow.

How to reproduce?

  1. Put Preprocess widget on the canvas
  2. Apply the Normalize pane
  3. Keep the Snowball preprocessor selected
  4. Change language for UDPIPE
  5. Save the workflow. When the workflow is reopened, the selected language for UDPIPE is not the same as the one used before (it is only remembered for the selected preprocessor).
Description of changes

Remember the setting for unselected preprocessors.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov-commenter
Copy link

codecov-commenter commented Mar 13, 2024

Codecov Report

Merging #1042 (3e374ff) into master (e36fdea) will decrease coverage by 0.17%.
The diff coverage is 100.00%.

❗ Current head 3e374ff differs from pull request most recent head cc7f4d0. Consider uploading reports for the commit cc7f4d0 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1042      +/-   ##
==========================================
- Coverage   82.51%   82.34%   -0.17%     
==========================================
  Files          92       93       +1     
  Lines       12400    12436      +36     
  Branches     1695     1691       -4     
==========================================
+ Hits        10232    10241       +9     
- Misses       1855     1886      +31     
+ Partials      313      309       -4     

@PrimozGodec PrimozGodec force-pushed the preprocess-fix-settings branch from 3e374ff to cc7f4d0 Compare March 14, 2024 10:35
@PrimozGodec PrimozGodec marked this pull request as ready for review March 14, 2024 10:36
@PrimozGodec PrimozGodec requested a review from VesnaT March 14, 2024 10:42
@PrimozGodec PrimozGodec marked this pull request as draft March 22, 2024 12:42
@PrimozGodec
Copy link
Collaborator Author

As discussed with @VesnaT, it is not correct way of solving the issue. The edited signal should be emitted only when the preprocessor is active since it causes the preprocessor to be applied to the data. It should be the change signal, which should cause the settings to be changed, but it must be fixed in the Preprocess widget on Orange, so I am closing this issue.

@PrimozGodec PrimozGodec deleted the preprocess-fix-settings branch March 27, 2024 08:53
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.

2 participants