The Wayback Machine - https://web.archive.org/web/20210823040838/https://github.com/scikit-learn/scikit-learn/pull/18410
Skip to content

/ scikit-learn

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FIX accept uint8 X in hist-gbdt.predict #18410

Merged
merged 3 commits into from Sep 26, 2020
Merged

FIX accept uint8 X in hist-gbdt.predict #18410

merged 3 commits into from Sep 26, 2020

Conversation

@NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Sep 16, 2020

Fixes #18408
@NicolasHug
fixed dtype issue
1ceb679
@NicolasHug NicolasHug mentioned this pull request Sep 16, 2020
Closed
@NicolasHug
whatsnew
5c71553
@NicolasHug
NicolasHug reviewed Sep 16, 2020
View changes
sklearn/ensemble/_hist_gradient_boosting/gradient_boosting.py Outdated
is_binned = getattr(self, '_in_fit', False)
dtype = [X_BINNED_DTYPE] if is_binned else [X_DTYPE]
Comment on lines 648 to 649

This comment has been minimized.

Sign in to view
@NicolasHug

NicolasHug Sep 16, 2020
Author Member

in master, X would still be of dtype X_BINNED_DTYPE (pass-through) but is_binned would be False so we'd call _predict_from_numeric_data which can only accept X_DTYPE, hence the failure

@NicolasHug
dont pass lists
f84e745
@thomasjpfan
thomasjpfan approved these changes Sep 17, 2020
View changes
Copy link
Member

@thomasjpfan thomasjpfan left a comment

LGTM
sklearn/ensemble/_hist_gradient_boosting/tests/test_gradient_boosting.py
rng = np.random.RandomState(0)

X = rng.randint(0, 100, size=(10, 2)).astype(np.uint8)
y = rng.randint(0, 2, size=10).astype(np.uint8)

This comment has been minimized.

Sign in to view
@thomasjpfan

thomasjpfan Sep 17, 2020
Member

Does y need to be casted here?

Suggested change
y = rng.randint(0, 2, size=10).astype(np.uint8)
y = rng.randint(0, 2, size=10)

This comment has been minimized.

Sign in to view
@NicolasHug

NicolasHug Sep 17, 2020
Author Member

It's not what was causing the failure on master, but can't hurt to have it anyway?

This comment has been minimized.

Sign in to view
@thomasjpfan

thomasjpfan Sep 17, 2020
Member

Okay, lets keep it.

@lorentzenchr
lorentzenchr approved these changes Sep 25, 2020
View changes
Copy link
Member

@lorentzenchr lorentzenchr left a comment

LGTM
@NicolasHug
Copy link
Member Author

@NicolasHug NicolasHug commented Sep 26, 2020

wanna merge it @lorentzenchr ;) ?
@lorentzenchr lorentzenchr merged commit df61e9e into scikit-learn:master Sep 26, 2020
19 checks passed
19 checks passed
@lgtm-com
LGTM analysis: C/C++ No code changes detected
Details
@lgtm-com
LGTM analysis: JavaScript No code changes detected
Details
@lgtm-com
LGTM analysis: Python No new or fixed alerts
Details
ci/circleci: deploy Your tests passed on CircleCI!
Details
ci/circleci: doc Your tests passed on CircleCI!
Details
@circleci-artifacts-redirector
ci/circleci: doc artifact Link to 0/doc/_changed.html
Details
ci/circleci: doc-min-dependencies Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
@azure-pipelines
scikit-learn.scikit-learn Build #20200916.9 succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linting) Linting succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux py36_conda_openblas) Linux py36_conda_openblas succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux py36_ubuntu_atlas) Linux py36_ubuntu_atlas succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux pylatest_pip_openblas_pandas) Linux pylatest_pip_openblas_pandas succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux32 py36_ubuntu_atlas_32bit) Linux32 py36_ubuntu_atlas_32bit succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Linux_Runs pylatest_conda_mkl) Linux_Runs pylatest_conda_mkl succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Windows py36_pip_openblas_32bit) Windows py36_pip_openblas_32bit succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (Windows py37_conda_mkl) Windows py37_conda_mkl succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (macOS pylatest_conda_mkl) macOS pylatest_conda_mkl succeeded
Details
@azure-pipelines
scikit-learn.scikit-learn (macOS pylatest_conda_mkl_no_openmp) macOS pylatest_conda_mkl_no_openmp succeeded
Details
@lorentzenchr lorentzenchr changed the title [MRG] Fix accept uint8 X in hist-gbdt.predict FIX accept uint8 X in hist-gbdt.predict Sep 26, 2020
amrcode added a commit to amrcode/scikit-learn that referenced this pull request Oct 19, 2020
@NicolasHug @amrcode
FIX accept uint8 X in hist-gbdt.predict (scikit-learn#18410)
1d115e9 
* fixed dtype issue

* whatsnew

* dont pass lists
jayzed82 added a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
@NicolasHug @jayzed82
FIX accept uint8 X in hist-gbdt.predict (scikit-learn#18410)
a581fe2 
* fixed dtype issue

* whatsnew

* dont pass lists
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasjpfan thomasjpfan

@lorentzenchr lorentzenchr

Assignees
No one assigned
Labels
module:ensemble Waiting for Reviewer
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Data type mismatch problem when calling HistGradientBoostingClassifier.predict()
4 participants
@NicolasHug @thomasjpfan @lorentzenchr @cmarmo