Skip to content

Conversation

OmarManzoor
Copy link
Contributor

Reference Issues/PRs

Towards #24875

What does this implement/fix? Explain your changes.

  • Used memory views to replace cnp.ndarray
  • Did some black formatting
  • Removed the ctypedef and replaced them with the original cnp types
  • Used a 2d memory view for X in _predict_regression_tree_inplace_fast_dense

Any other comments?

@OmarManzoor OmarManzoor changed the title Cython gradient boosting MAINT Remove -Wcpp warnings when compiling sklearn.ensemble._gradient_boosting Nov 25, 2022
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@OmarManzoor
Copy link
Contributor Author

@jjerphan Could you kindly have a look at this PR?

Copy link
Member

@jjerphan jjerphan left a comment

Choose a reason for hiding this comment

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

LGTM.

@OmarManzoor
Copy link
Contributor Author

@glemaitre , @jjerphan Is this PR capable of being merged?

@jjerphan jjerphan merged commit 6367597 into scikit-learn:main Dec 15, 2022
@jjerphan
Copy link
Member

Thanks for the heads-up, @OmarManzoor.

@OmarManzoor OmarManzoor deleted the cython_gradient_boosting branch December 15, 2022 08:02
@OmarManzoor
Copy link
Contributor Author

OmarManzoor commented Dec 15, 2022

Thanks for the heads-up, @OmarManzoor.

Thank you for the quick merge 😃

@jjerphan
Copy link
Member

We often multitask between PRs and issues so it's easy to loose track of what is mergeable.

See #25069 for more context.

@OmarManzoor
Copy link
Contributor Author

We often multitask between PRs and issues so it's easy to loose track of what is mergeable.

See #25069 for more context.

Totally understandable. Thank you for your consideration.

jjerphan pushed a commit to jjerphan/scikit-learn that referenced this pull request Jan 3, 2023
…_boosting (scikit-learn#25033)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
jjerphan pushed a commit to jjerphan/scikit-learn that referenced this pull request Jan 20, 2023
…_boosting (scikit-learn#25033)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
jjerphan pushed a commit to jjerphan/scikit-learn that referenced this pull request Jan 20, 2023
…_boosting (scikit-learn#25033)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants