8000 DOC docstring (shape= -> of shape) by adrinjalali · Pull Request #14640 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

DOC docstring (shape= -> of shape) #14640

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.

Sign up for GitHub

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
Merged
merged 45 commits into from
Oct 3, 2019
Merged

DOC docstring (shape= -> of shape) #14640

merged 45 commits into from
Oct 3, 2019

Conversation

adrinjalali
Copy link
Member

This PR makes our docstrings a bit more compliant with our brand-new style (#12356)

@adrinjalali
some sed-ing
fb9321d
@adrinjalali
replace array-like, shape = (n_samples,)
696cf7e
@adrinjalali
array-like, shape = [n_samples] or [n_samples, n_output]
7d1d5ab
@adrinjalali
fix of shape of previous commit
06a0e8b
@adrinjalali
fix array-like, shape = [n_samples] or [n_samples, n_targets]
9bc35e4
@adrinjalali
fix array-like, shape = [n_samples, n_features]
73cdb2b
@adrinjalali
array-like, shape = [n_samples, ], [n_samples, n_classes]
98d968c
@adrinjalali
array-like, shape = [n_samples, n_transformed_features]
c258fca
@adrinjalali
array-like, shape = [n_classes, n_features]
45ff9d7
@adrinjalali
array-like, shape = [n_samples] or [n_samples, n_outputs]
b23a027
@adrinjalali
array-like, shape = (n_samples, n_features)
412b695
@adrinjalali
array-like, shape = (n_samples, [n_output_dims])
5e93e4d
@adrinjalali
small ones
e9fe2ca
@adrinjalali
array-like, shape = [n_samples, n_classes]
3480e29
@adrinjalali
array-like, shape = [n_samples, n_targets]
f1afb3f
@adrinjalali
array-like, shape = [n_samples] or None
284ebfd
@adrinjalali
sample_weights
a9747fd
@adrinjalali
(n_samples,)
e8b53bf
@adrinjalali
array-like, shape = [n_samples]
40006dc
@adrinjalali
n_SV
b0072ff
@adrinjalali
leftovers
7d4160f
@adrinjalali adrinjalali changed the title Doc/array shape DOC docstring (shape= -> of shape) Aug 13, 2019
@adrinjalali adrinjalali changed the title DOC docstring (shape= -> of shape) [MRG] DOC docstring (shape= -> of shape) Aug 13, 2019
glemaitre
glemaitre approved these changes Aug 13, 2019
View reviewed changes
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.

LGTM. Single question: you make the sample_weight docstring consistent but you removed the default=None. I was checking the documentation but this is not mentioned. What is the policy for the default in this case.

@adrinjalali
Copy link
Member Author

I didn't really try to touch the default much, but to me array-like of shape (n_samples,) or None kinda implicitly says that the default is None, don't you think? Also, I think we may end up fixing those rules as we go along and we encounter cases where the rules don't make much sense, maybe?

@glemaitre
Copy link
Member

OK it makes sense to me. We might just think about adding these into the contributing guide we are settled.

@thomasjpfan
Copy link
Member

I see three options with handling default=None:

  1. array-like of shape (n_samples,) or None, default=None

  2. array-like of shape (n_samples,) or None

  3. array-like of shape (n_samples,), default=None

@adrinjalali
Copy link
Member Author

I kinda like option 3

@adrinjalali
Copy link
Member Author

I think I don't have permissions to do that @glemaitre , I'll send another commit to get them run.

@adrinjalali
Copy link
Member Author

@glemaitre they pass now :)

thomasjpfan
thomasjpfan reviewed Sep 28, 2019
View reviewed changes
Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

What do you think of adding a small test where if shape is in parameters docstring it must match the following regex:

of shape \([\w, ]+\)

@adrinjalali
Copy link
Member Author

What do you think of adding a small test where if shape is in parameters docstring it must match the following regex:

of shape \([\w, ]+\)

makes sense to me, I'd just add a .* at the end of that.

@glemaitre
Copy link
Member

Still my +1. You can add a test if you want.

@adrinjalali
Copy link
Member Author

I rather have the test in a separate PR though.

@glemaitre
Copy link
Member

Fine with me

@thomasjpfan thomasjpfan changed the title [MRG] DOC docstring (shape= -> of shape) DOC docstring (shape= -> of shape) Oct 3, 2019
@thomasjpfan thomasjpfan merged commit 0eebade into scikit-learn:master Oct 3, 2019
@adrinjalali adrinjalali deleted the doc/array-shape branch October 4, 2019 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasjpfan thomasjpfan thomasjpfan approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@adrinjalali @glemaitre @thomasjpfan @amueller @NicolasHug
0