biolab / orange3-timeseries

🍊 :chart_with_upwards_trend: Orange add-on for analyzing, visualizing, manipulating, and forecasting time series data.
Other
62 stars 41 forks source link

Model Evaluation: Use deferred commit #218

Closed janezd closed 2 years ago

janezd commented 2 years ago
Issue

Model Evaluation did not use deferred commit.

Description of changes

Model Evaluation uses deferred commit.

Includes
codecov-commenter commented 2 years ago

Codecov Report

Merging #218 (1fc78fb) into master (6792489) will increase coverage by 1.40%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #218      +/-   ##
==========================================
+ Coverage   72.19%   73.60%   +1.40%     
==========================================
  Files          29       30       +1     
  Lines        4050     4107      +57     
  Branches      626      636      +10     
==========================================
+ Hits         2924     3023      +99     
+ Misses        992      947      -45     
- Partials      134      137       +3     
Impacted Files Coverage Δ
...ngecontrib/timeseries/widgets/owmodelevaluation.py 94.44% <100.00%> (-0.08%) :arrow_down:
orangecontrib/timeseries/timeseries.py 88.20% <0.00%> (-2.81%) :arrow_down:
orangecontrib/timeseries/functions.py 76.38% <0.00%> (-0.19%) :arrow_down:
orangecontrib/timeseries/widgets/owperiodbase.py 96.73% <0.00%> (ø)
orangecontrib/timeseries/widgets/owcorrelogram.py 97.82% <0.00%> (+2.30%) :arrow_up:
orangecontrib/timeseries/widgets/owdifference.py 100.00% <0.00%> (+3.53%) :arrow_up:
orangecontrib/timeseries/widgets/owperiodogram.py 71.42% <0.00%> (+71.42%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6792489...1fc78fb. Read the comment docs.

ajdapretnar commented 2 years ago

Just a question. Is Apply really necessary here? Test and Score doesn't have one and this widget is conceptually similar to it. For example, if I change the number of folds and press Enter, the changes will be applied irrespective of the status of the button. The only time the button matters is when the user doesn't confirm spin box values by pressing Enter. A thought.

janezd commented 2 years ago

I think the button is needed to prevent recomputation at every step. :(

Test & Score would in principle need it, too. But Test & Score doesn't use spin for number of folds, and in Test & Score you choose a different option and expect immediate recomputation (because there are no other changes you'd like to make before). So I think we can have Test and Score without the button, but this widget needs it. (Even if we change the first spin into combo.)

ajdapretnar commented 2 years ago

Ok, then I am merging.