Don't fill first timestamps in TimeSeriesImputerTransform
#634
Conversation
Mr-Geekman
added
bug
Codecov Report
@@ Coverage Diff @@
## master #634 +/- ##
=======================================
Coverage 84.26% 84.27%
=======================================
Files 118 118
Lines 6242 6243 +1
=======================================
+ Hits 5260 5261 +1
Misses 982 982
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
| @@ -93,12 +99,6 @@ def transform(self, df: pd.DataFrame) -> pd.DataFrame: | |||
| result_df = df.copy() | |||
| cur_nans = result_df[result_df[self.in_column].isna()].index | |||
|
|
|||
| # check if all values are nans | |||
| if cur_nans.shape[0] == result_df.shape[0] and self.strategy != ImputerMode.zero: | |||
There was a problem hiding this comment.
are you sure we can drop this check?
how about transform call in TSDataset.make_future?
There was a problem hiding this comment.
In make_future we have train_values + future_values. If all values are NaNs in make_future then on fit stage they also was NaN (because there was only train_values) and we will get an exception on fit stage.
However, if in the future we will make a transform in make_future on part of train data (for optimization) we can face the situation when we have non-nan values on fit, but all nans on transform. But this whole situation looks troublesome: we want to make imputation by train values and we have non-nans train values on fit, but we lost them on transform stage and can't make a transformation. That means that we've already made a mistake by this separation of data on fit and transform and this mistake isn't really a problem of our transform.
|
🚀 Deployed on https://deploy-preview-634--etna-docs.netlify.app |
IMPORTANT: Please do not create a Pull Request without creating an issue first.
Before submitting (must do checklist)
Type of Change
Proposed Changes
Look #601, this is the implementation of the second solution.
Related Issue
#601.
Closing issues
Closes #601.