kz930 opened a new pull request, #7598:
URL: https://github.com/apache/texera/pull/7598
### What changes were proposed in this PR?
`SklearnAdvancedKNNParameters` pairs each hyperparameter with the Python
callable that converts what the user typed. Two of the seven named one that
cannot produce what scikit-learn accepts, so picking either failed the run
whatever was entered, from the same dropdown as the five that work. Both
trainers share the enum, so both were affected.
`metric` was declared `int`. Its accepted values are words, so
`int("minkowski")` raises before scikit-learn sees anything, and a number that
does convert is rejected as not one of the accepted names. It is now `str`,
which `weights` and `algorithm` beside it already are.
`metric_params` takes a mapping of extra keyword arguments for the metric,
and none of `int`, `float` or `str` returns one, so a well-formed `{"p": 2}`
arrived as that same text. It now names `json.loads`. The type has never been
limited to builtins, since the SVC and SVR trainers already name an inline
lambda for their boolean parameters, but `json.loads` needs the module, so the
generated template imports json. Unconditionally rather than when such a
parameter is present: the alternative is threading each converter's imports
through every `ParamClass` for one parameter of one operator.
### Any related issues, documentation, discussions?
Fixes #7593.
### How was this PR tested?
- Reproduced against scikit-learn both ways. Before: `int("minkowski")`
raises ValueError, and `metric=int("3")` reaches the estimator and is rejected
as not an accepted metric name. After: minkowski with `metric_params` `{"p":
3}` fits and predicts with the mapping arriving as a dict, chebyshev fits on
the regressor, and mahalanobis with a VI matrix fits, which is the case
`metric_params` exists for.
- The KNN classifier spec asserts which callable each value is handed to in
the emitted model call, not only in the parameter summary beside it. Values
travel base64-encoded, so the assertion is on the converter rather than the
literal.
- The base descriptor's spec covers the template's new import.
- `WorkflowOperator/testOnly *sklearnAdvanced*`: 7 suites, 40 tests, none
failed. `scalafmtCheckAll` and both scalafix checks clean.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
