Closed janezd closed 2 years ago
Most constructors for Timeseries
are patched through Timeseries.from_data_table
, which construct a view. This is mostly unnecessary in happens only for convience. However, the result is that those table can't be unlocked.
@markotoplak, what you said on Friday can really bite. In this particular case, though, I think it should only affect tests.
In general, we should strive to write widgets so that they never modify a table once it's created. Patching should be limited to tests.
Merging #195 (9b70f11) into master (3ce4ca1) will increase coverage by
0.76%
. The diff coverage is91.66%
.
@@ Coverage Diff @@
## master #195 +/- ##
==========================================
+ Coverage 72.43% 73.20% +0.76%
==========================================
Files 7 7
Lines 809 821 +12
Branches 133 161 +28
==========================================
+ Hits 586 601 +15
+ Misses 171 168 -3
Partials 52 52
Impacted Files | Coverage Δ | |
---|---|---|
orangecontrib/timeseries/timeseries.py | 85.95% <87.50%> (+0.83%) |
:arrow_up: |
orangecontrib/timeseries/functions.py | 77.09% <100.00%> (ø) |
|
orangecontrib/timeseries/models.py | 69.60% <100.00%> (+1.78%) |
: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 14a557f...9b70f11. Read the comment docs.
@markotoplak, can you review this? In particular because of https://github.com/biolab/orange3-timeseries/pull/195/commits/7ef044f31ab11d2cf80aa15f0bbcc5a80308c44e.
Changes are explained in bullets in description.
Merging this (if correct) is necessary to continue any work on this add-on.
Issue
There have been multiple changes in statsmodels, as well as in Orange, for instance table locking.
Also fixes #196.
Description of changes
TimeSeries
was unlockable because it was constructed as a view into an intermediate tableautocorrelation
was removed because it is no longer available; there have been some arguments against "unbiasing" models in this way.freq
has been renamed toperiod
; meaning is the same, I've found thestatsmodels
's commit in which they renamed itndim
and if it was1
, printed a warning "something went wrong". 🤯 I added a missing "err" (which was, as I see, all that would need to be done in #37!, but then still kept a rephrased warning about errored model(s).Includes