Update support_vector_machines.py to add the polynomial kernel - #12748
faefabsdgd wants to merge 3 commits into
Conversation
for more information, see https://pre-commit.ci
|
@priya-sundaram-dev, please review. Do the proposed modifications add real value? The new tests only seem to be about new exceptions being raised, not about any new functionality. |
|
Reviewed. Short answer: yes, this adds real value — the polynomial kernel But your instinct is right, @cclauss — the tests demonstrate nothing about the new functionality. All three new doctests only assert that bad arguments raise. There is no doctest showing the kernel actually computes or that an Concrete asks for @faefabsdgd before merge:
Nothing here is a blocker on the idea — it's a legitimate addition. It just needs a test that exercises the kernel, not only the exceptions. Happy to re-review once (1) is in. |
|
@algorithms-keeper review |
|
@cclauss Reviewed. Short version: yes, it adds real value, with one nit. Value: the polynomial kernel On your tests observation: you're right that the three new doctests only assert exceptions ( What would make it clearly worth merging (raise the bar for the whole file, starting here): add one functional doctest that exercises the kernel end-to-end — e.g. build a tiny 2-class dataset, Minor: Advice: request the one functional doctest, then merge. |
Describe your change:
Checklist: