Skip to content

DOC Ensures that function passes numpydoc validation: paired_distances#22380

Merged
glemaitre merged 4 commits intoscikit-learn:mainfrom
reshamas:np_pair_dist
Feb 11, 2022
Merged

DOC Ensures that function passes numpydoc validation: paired_distances#22380
glemaitre merged 4 commits intoscikit-learn:mainfrom
reshamas:np_pair_dist

Conversation

@reshamas
Copy link
Copy Markdown
Member

@reshamas reshamas commented Feb 4, 2022

Reference Issues/PRs

Addresses #21350.
Closes #21440.

What does this implement/fix? Explain your changes.

Updates docstring in function to fix numpydoc errors.

Any other comments?

#DataUmbrella sprint

@reshamas
Copy link
Copy Markdown
Member Author

reshamas commented Feb 4, 2022

@glemaitre I was not sure how to resolve this numpy doc error:

E           
E           # Errors
E           
E            - PR01: Parameters {'**kwds'} not documented

sklearn/tests/test_docstrings.py:364: ValueError
============================================= 1 failed, 2107 deselected in 1.03s =============================================
(sklearndev) 

@@ -1000,9 +1000,9 @@ def paired_cosine_distances(X, Y):

def paired_distances(X, Y, *, metric="euclidean", **kwds):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You are missing a docstring for the **kwds argument

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks @glemaitre. Added the docstring for that now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So we decided to not deprecate and remove this parameter.
We can document it and mentioned that it is not used at the moment.

@reshamas
Copy link
Copy Markdown
Member Author

reshamas commented Feb 7, 2022

Reminder from discussion with @glemaitre:
open up an issue:
Deprecate the parameter **kwds in function def paired_distances since it is unused:
def paired_distances(X, Y, *, metric="euclidean", **kwds):

Comment on lines +1027 to +1029
**kwds : dict
Unused parameters.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@glemaitre
Is this text ok?

@glemaitre glemaitre merged commit 0cb06ea into scikit-learn:main Feb 11, 2022
@glemaitre
Copy link
Copy Markdown
Member

Thanks @reshamas, the changes are good.

thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Mar 1, 2022
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