Covariance kernels #601
Covariance kernels #601
Conversation
Converted tests from old test_covariances file to pytest format and added tests for functional data objects.
Adapted Linear, Polynomial, Exponential, Gaussian and Matern covariances and added control messages for covariance kernels that do not support functional data objects.
Changed function to test multivariate data for all covariance functions. Now uses ParameterGrid.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
pep8
skfda/misc/covariances.py|822 col 1| D102 Missing docstring in public method
skfda/misc/covariances.py|865 col 1| D102 Missing docstring in public method
skfda/misc/covariances.py|872 col 1| W293 blank line contains whitespace
skfda/misc/covariances.py|873 col 9| WPS428 Found statement that has no effect
skfda/misc/covariances.py|873 col 9| WPS462 Wrong multiline string usage
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #601 +/- ##
===========================================
- Coverage 86.65% 86.65% -0.01%
===========================================
Files 156 156
Lines 13314 13356 +42
===========================================
+ Hits 11537 11573 +36
- Misses 1777 1783 +6 ☔ View full report in Codecov by Sentry. |
skfda/tests/test_covariances.py
Outdated
| fd = fetch_functional_data | ||
| cov_kernel = covariances_raise_fixture | ||
|
|
||
| np.testing.assert_raises( |
There was a problem hiding this comment.
It is better to use pytest.raises as a context manager: https://docs.pytest.org/en/7.1.x/how-to/assert.html#assertions-about-expected-exceptions
There was a problem hiding this comment.
It was meant to be used as a context manager (in a with statement).
|
What is the status of this (and subsequent) PRs? Do you have time to take a look at my comments now, @E105D104U125, so that we can move forward and merge this? |
There was a problem hiding this comment.
First, I apologize for taking so long on reviewing this PR. I had a lot of work to do and very few time to dedicate to this.
I hope I will have more time now, so if you are busy and/or do not want to make the changes to merge this PR, just tell me and I will try to make them on my own and merge it.
| y: Input | None = None, | ||
| ) -> NDArrayFloat: | ||
| """Compute white noise covariance function on input data.""" | ||
| if isinstance(x, FData) or isinstance(y, FData): |
There was a problem hiding this comment.
Maybe then it is better to restrict it in the type hints, like it was done for the Brownian.
| @@ -808,6 +903,9 @@ def __call__(self, x: ArrayLike, y: ArrayLike) -> NDArrayFloat: | |||
| Returns: | |||
| Covariance function evaluated at the grid formed by x and y. | |||
| """ | |||
| if isinstance(x, FData) or isinstance(y, FData): | |||
There was a problem hiding this comment.
Same as above: restrict it in the type hints.
Changed covariances.py from misc module in order to generalize the Linear, Polynomial, Exponential, Gaussian and Matern kernels for FData.
Included tests for theses changes and adapted the already existing tests for multivariate data to pytest format.