Update to latest versions of sklearn and scipy #239
Conversation
PicoCentauri
force-pushed
the
fix-sklearn
branch
2 times, most recently
from
75e03da to
879057b
Compare
PicoCentauri
force-pushed
the
fix-sklearn
branch
from
879057b to
feb10b0
Compare
PicoCentauri
force-pushed
the
fix-sklearn
branch
from
0a4af31 to
790eee3
Compare
* replacing `self._validate_data` with `validate_data` * Additional fixes * Fixing scikit-learn warnings * Fixing PCovR to work with 1D column vectors * Fixing examples * Changing shape of y in pcovr tests * Fixing PCovR to not mess with data shape --------- Co-authored-by: Christian Jorgensen <[email protected]>
PicoCentauri
changed the title
src/skmatter/_selection.py
Outdated
| y : ignored | ||
|
|
||
| Returns | ||
| ------- | ||
| score : numpy.ndarray of (n_to_select_from_) | ||
| :math:`\pi` importance for the given samples or features | ||
| """ | ||
| if y is not None: | ||
| validate_data(self, X, y.ravel(), reset=False) |
There was a problem hiding this comment.
With the checks already present in the superclass, I do not think these validations are necessary.
There was a problem hiding this comment.
Yep. We need to keep the validation for X even though it's not necessary because scikit-learn now enforces that the shape of X passed for score() must be consistent with the shape of X used in fit(). The validation for y is not necessary and I have removed it.
src/skmatter/_selection.py
Outdated
| if y is not None: | ||
| validate_data(self, X, y.ravel(), reset=False) | ||
| else: | ||
| validate_data(self, X, reset=False) # present for API consistency |
There was a problem hiding this comment.
Same note as above. These matrices will already be validated in GreedySelector.
src/skmatter/_selection.py
Outdated
| if y is not None: | ||
| validate_data(self, X, y.ravel(), reset=False) | ||
| else: | ||
| validate_data(self, X, reset=False) |
There was a problem hiding this comment.
Same note as above. These matrices will already be validated in GreedySelector.
src/skmatter/_selection.py
Outdated
| if y is not None: | ||
| validate_data(self, X, y.ravel(), reset=False) | ||
| else: | ||
| validate_data(self, X, reset=False) |
There was a problem hiding this comment.
Same note as above. These matrices will already be validated in GreedySelector.
Co-authored-by: Rose K. Cersonsky <[email protected]>
Co-authored-by: Rose K. Cersonsky <[email protected]>
Fixes #237
Reopening of #238
Adding fixes for updates with scikit-learn and scipy. Scikit-learn's estimator checks will fail when
validate_datais not used. These fixes will pass scikit-learn's estimator checks but are causing unit tests to fail.Contributor (creator of PR) checklist
For Reviewer
📚 Documentation preview 📚: https://scikit-matter--239.org.readthedocs.build/en/239/