[ENH] add a difference transformer to series transformations #2729
Conversation
aeon-actions-bot
bot
added
enhancement
Thank you for contributing to
|
|
It seems that one of the automated checks failed with errors indicating it couldn't find the merge base or my commit SHA: |
aeon/transformations/series/_diff.py
Outdated
| if not isinstance(order, int) or order < 1: | ||
| raise ValueError(f"`order` must be a positive integer, but got {order}") | ||
| self.order = order | ||
| super().__init__(axis=axis) |
There was a problem hiding this comment.
Remove the axis parameter. This is used to determine the shape of the time series internally. We only want to apply this to series, not between channels.
There was a problem hiding this comment.
The "axis" is inherited from BaseSeriesTransformer. Should "axis = 1" be used to indicate that the time series are all in rows, with shape (n_channels, n_timepoints)?
There was a problem hiding this comment.
yes, thats telling the base class to convert the series to (n_channels, n_timepoints) before passing it to _fit and other functions
aeon/transformations/series/_diff.py
Outdated
| if not isinstance(order, int) or order < 1: | ||
| raise ValueError(f"`order` must be a positive integer, but got {order}") |
There was a problem hiding this comment.
I would do this in fit
|
|
||
| dt1 = DifferenceTransformer(order=1) | ||
| Xt1 = dt1.fit_transform(X) | ||
| expected1 = np.array([[np.nan, 3.0, 5.0, 7.0, 9.0, 11.0]]) |
There was a problem hiding this comment.
IMO better to return a smaller series than include NaNs. Possibly a parameter if you think it is worth it but by default change the shape.
|
|
||
| def test_diff(): | ||
| """Tests basic first and second order differencing.""" | ||
| X = np.array([[1.0, 4.0, 9.0, 16.0, 25.0, 36.0]]) |
There was a problem hiding this comment.
Can you also test multivariate series in another test perhaps
|
I have made som modifications according to your comments. @MatthewMiddlehurst |
Reference Issues/PRs
fixes #1553
What does this implement/fix? Explain your changes.
This implements a transformer that computes the n-order differences of a time series.
The input time series is expected to have a NumPy inner-type, and the transformation is performed using numpy.diff().
To preserve the original shape of the series along the time axis, the first order element(s) of the output are filled with NaN.
Does your contribution introduce a new dependency? If yes, which one?
No. It only depends on NumPy.
Any other comments?
No.
PR checklist
For all contributions
For new estimators and functions
__maintainer__at the top of relevant files and want to be contacted regarding its maintenance. Unmaintained files may be removed. This is for the full file, and you should not add yourself if you are just making minor changes or do not want to help maintain its contents.For developers with write access