Skip to content

Added automatic validation function for sklearn.neighbors.radius_neighbors_graph (#24862) #27245

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 13 commits into from
Sep 5, 2023

Conversation

sqali
Copy link
Contributor

@sqali sqali commented Aug 30, 2023

Hi @jeremiedbb , @glemaitre

This pull request is raised to add an automatic validation function for sklearn.neighbors.radius_neighbors_graph based on issue #24862. I have also updated the test_public_functions. Kindly check and review the changes made and let me know if they require any modifications.

Thanks & Regards

Saed Qaiser Ali

sqali added 2 commits August 30, 2023 22:31
Added validation params to sklearn.neighbors.radius_neighbors_graph
Added sklearn.neighbors.radius_neighbors_graph to the test_public_functions list
@github-actions
Copy link

github-actions bot commented Aug 30, 2023

✔️ Linting Passed

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

Generated for commit: 5bf1c77. Link to the linter CI: here

corrected a comma
@sqali sqali changed the title Added automatic validation function for sklearn.neighbors.radius_neighbors_graph Added automatic validation function for sklearn.neighbors.radius_neighbors_graph (#24862) Aug 30, 2023
@sqali
Copy link
Contributor Author

sqali commented Aug 31, 2023

Hi @jeremiedbb @glemaitre ,

All the checks have passed, Kindly review the changes made and let me know if there are any modifications to be done.

Thanks & Regards

Sayed Qaiser Ali

Copy link
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @sqali ! Here are a few comments

@jeremiedbb jeremiedbb added No Changelog Needed Validation related to input validation labels Sep 1, 2023
sqali and others added 4 commits September 1, 2023 18:33
removed changelogs
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
@sqali
Copy link
Contributor Author

sqali commented Sep 1, 2023

Thanks so much for the guidance @jeremiedbb. I have made the changes as you suggested. Kindly review it.

Copy link
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

I directly pushed to fix the docstring. LGTM. Thanks @sqali

@sqali
Copy link
Contributor Author

sqali commented Sep 4, 2023

Hi @jeremiedbb ,

I am unable to see the changes, is there some issue?

@jeremiedbb jeremiedbb merged commit 1924ffb into scikit-learn:main Sep 5, 2023
@jeremiedbb
Copy link
Member

I am unable to see the changes

What do you mean by that ? Until we merge the PR, the changes are only visible in the PR. Once we merge it (which I just did) then changes are visible in the main branch of scikit-learn.

glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 5, 2023
…s_neighbors_graph (scikit-learn#27245)

Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
…s_neighbors_graph (scikit-learn#27245)

Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.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.

2 participants