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] Formula: Remove context settings #6801

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

ales-erjavec
Copy link
Contributor

@ales-erjavec ales-erjavec commented May 10, 2024

Issue

Fixes: gh-5636

Description of changes

Remove context settings from Formula widget.

Includes
  • Code changes
  • Tests
  • Documentation

Copy link

codecov bot commented May 10, 2024

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.24%. Comparing base (8831a57) to head (740b823).
Report is 3 commits behind head on master.

Current head 740b823 differs from pull request most recent head 5c0cf8a

Please upload reports for the commit 5c0cf8a to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6801      +/-   ##
==========================================
- Coverage   88.26%   88.24%   -0.03%     
==========================================
  Files         326      327       +1     
  Lines       71156    71331     +175     
==========================================
+ Hits        62805    62944     +139     
- Misses       8351     8387      +36     

@ales-erjavec ales-erjavec force-pushed the feature-constructor-no-context branch 3 times, most recently from 740b823 to 5dc34aa Compare May 31, 2024 07:48
@ales-erjavec ales-erjavec marked this pull request as ready for review May 31, 2024 08:17
@ales-erjavec ales-erjavec force-pushed the feature-constructor-no-context branch from 5dc34aa to 8cbf2dc Compare May 31, 2024 08:21
@ales-erjavec ales-erjavec force-pushed the feature-constructor-no-context branch from 8cbf2dc to a60fb48 Compare June 4, 2024 11:28
@janezd janezd self-assigned this Jun 7, 2024
@janezd
Copy link
Contributor

janezd commented Jun 20, 2024

I like the idea, it will solve a lot of problems.

I wrote two formulas, one with petal_length and one with petal_length and petal_width. Then I removed petal_width. All formulas persisted, the widget reported the error (via transform_error, I believe) and did not update the output, so it still output the data from the previous input. I think the widget should do one of the following:

  • remove formulae that miss some variables from the model, but keep them in settings as "hints" so they reappear if another input data again contains them,
  • show a warning and mark the formulae that cannot be computed (or report them in the warning description) due to missing variables or another problem, but compute the other formulae and update the output,
  • report an error (as it does currently, just perhaps without outputting the exception in the console, if possible) and set output to None.

@janezd janezd removed their assignment Jun 20, 2024
@janezd janezd merged commit 7e620b6 into biolab:master Jun 26, 2024
16 of 28 checks passed
stuart-cls pushed a commit to stuart-cls/orange3 that referenced this pull request Nov 5, 2024
owfeatureconstructor: Remove domain context settings
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.

Feature constructor forgets everything when variable change in upstream widgets
2 participants