Repository navigation
Conversation
MLModel.predict() always cast classification results to np.int_, which raises a ValueError for any classifier trained on string class labels (e.g. the standard iris dataset's species names). Try the int cast first for backward compatibility, since Elasticsearch can return numeric class labels as numeric-looking strings, and only fall back to letting numpy infer the dtype when the cast actually fails. Fixes elastic#434
|
💚 CLA has been signed |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #434.
predict()always casts classification results tonp.int_, which blows up for any classifier trained on non-numeric class labels. The iris dataset's species names are the obvious example:Fix tries the
np.int_cast first, then falls back to letting numpy infer the dtype if that cast raises. Kept the try-first order on purpose: turns out Elasticsearch can return numeric class labels as numeric-looking strings (depending on how the model got serialized), so a plain "is this a string" check would have broken the existing int-labeled classifier tests. Found that out the hard way while testing this against a real cluster.Verified against a real Elasticsearch 9.2.0 instance, not mocked:
RandomForestClassifieron iris with the actual string species names, imported it, watchedpredict()throw the ValueError above pre-fix, then confirmed it returns string predictions matching sklearn's own output post-fix.test_decision_tree_classifiercases (all 4 parametrizations) to make sure numeric-labeled classifiers still come back asnp.int_. No regression there.test_decision_tree_classifier_string_class_labelsnext to them, same pattern, covering the case from the issue.Also fixed the
predict()docstring, it still claimed results are always float or int.