Skip to content

Replaced unsigned char with uint8_t in Cython Codebase (9 files) #29597

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

Merged
merged 5 commits into from
Aug 1, 2024
Merged

Replaced unsigned char with uint8_t in Cython Codebase (9 files) #29597

merged 5 commits into from
Aug 1, 2024

Conversation

norgera
Copy link
Contributor

@norgera norgera commented Jul 31, 2024

Replace unsigned char with uint8_t in Cython Codebase

Reference Issues/PRs

Fixes #29588

Files Updated

./sklearn/metrics/_pairwise_distances_reduction/_radius_neighbors_classmode.pyx.tp
./sklearn/ensemble/_hist_gradient_boosting/common.pxd
./sklearn/ensemble/_hist_gradient_boosting/_bitset.pyx
./sklearn/ensemble/_hist_gradient_boosting/_predictor.pyx
./sklearn/ensemble/_hist_gradient_boosting/splitting.pyx
./sklearn/ensemble/_hist_gradient_boosting/histogram.pyx
./sklearn/ensemble/_hist_gradient_boosting/_binning.pyx
./sklearn/ensemble/_hist_gradient_boosting/_bitset.pxd
./sklearn/linear_model/_sgd_fast.pyx.tp

Motivation

Using uint8_t instead of unsigned char makes the code more explicit and easier to reason about the range of admissible values (0-255). This change aligns with the recent efforts to use consistent type definitions throughout the codebase.

Copy link

github-actions bot commented Jul 31, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: b6b4809. Link to the linter CI: here

@norgera norgera changed the title Replace unsigned char with uint8_t in _radius_neighbors_classmode.pyx.tp Replaced unsigned char with uint8_t in Cython Codebase (9 files) Jul 31, 2024
@adam2392
Copy link
Member

LGTM

@adam2392 adam2392 added the Waiting for Second Reviewer First reviewer is done, need a second one! label Jul 31, 2024
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@adam2392 adam2392 enabled auto-merge (squash) August 1, 2024 13:22
@adam2392
Copy link
Member

adam2392 commented Aug 1, 2024

Enabled auto-merge. Thanks for the PR @norgera!

@adam2392 adam2392 merged commit 21e1642 into scikit-learn:main Aug 1, 2024
28 checks passed
MarcBresson pushed a commit to MarcBresson/scikit-learn that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cython module:metrics Waiting for Second Reviewer First reviewer is done, need a second one!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MAINT Replace unsigned char with uint8_t in codebase
3 participants