[MRG] FIX Nystroem with precomputed kernel #14706
Conversation
LGTM Please also add a bugfix entry in |
can you add a whatsnew? |
actually, not sure if this is the right fix, maybe |
Now we can use the |
@amueller if self.kernel == 'precomputed':
return params
if not callable(self.kernel):
for param in (KERNEL_PARAMS[self.kernel]):
if getattr(self, param) is not None:
params[param] = getattr(self, param)
else:
if (self.gamma is not None or
self.coef0 is not None or
self.degree is not None):
raise ValueError("Don't pass gamma, coef0 or degree to "
"Nystroem if using a callable kernel.") |
Right, if someone wants to iterate over |
Please correct me If I am wrong, (self.gamma is not None or
self.coef0 is not None or
self.degree is not None) when
On a side note, I saw here, we have been using this inside 'precomputed': None, # HACK: precomputed is always allowed, never called That is the reason, why I had pursued ahead to add |
I feel if we can avoid the hack that's better ;) |
ya, makes sense. Thanks for the input. Co-Authored-By: Tom Dupré la Tour <[email protected]>
Co-Authored-By: Tom Dupré la Tour <[email protected]>
Please also test that the correct error is raised with a precomputed kernel and passed kernel parameters. |
Thanks a lot @adrinjalali for your valuable comments. Please review my latest commit. |
LGTM, thanks @venkyyuvy |
e55e37c
into
scikit-learn:master
Nice, thanks ! |
Reference Issues/PRs
Fixes #14641
What does this implement/fix? Explain your changes.
Added
precomputed
inKERNEL_PARAMS
Nystroem
withprecomputed
kernel usingpolynomial_kernel
Any other comments?