DOC Update docs guideline regarding docstring formatting #18243

glemaitre · 2020-08-23T07:52:28Z

We start to use the following convention and we should mention it in the documentation guideline:

multioutput_array : ndarray of (n_samples, n_classes) or list of thereof

of thereof refer to a container of the previous type.

alfaro96

I would be happy to see this across the documentation!

doc/developers/contributing.rst

thomasjpfan · 2020-08-23T14:27:09Z

doc/developers/contributing.rst

@@ -734,7 +736,8 @@ In general have the following in mind:
    5. Specify ``dataframe`` when "frame-like" features are being used, such
       as the column names.
    6. When specifying the data type of a list, use ``of`` as a delimiter:
-       ``list of int``.
+       ``list of int``. If the data type was also mentioned as possible output,
+       you can use ``list of thereof`` to be more concise.


I am unable to easily parse "list of thereof". What is thereof referencing to?

int or list of thereof -> int or list of int
ndarray of shape (n_samples, n_features), dtype=np.int64 or list of thereof -> ndarray of shape (n_samples, n_features), dtype=np.int64 or list of ndarray of shape (n_samples, n_features), dtype=np.int64

If I recall @NicolasHug mentioned that Django (or was it another library?) are using this way for this particular case?

Cross reference: #17306 (comment)

It should be "of thereof" I believe.

And I think we should give a more concrete example:

For example if a parameter supports ints, floats, lists of ints, and list of float, you can use "int, floats, or list thereof".

Also I don't think using "thereof" is warranted when it refers to only one simple thing: "int or list thereof" should probably just be "list or list of int"

Then again, not a native speaker...

I see what you mean now. As a native speaker, I feel that this phrasing will be confusing.

In any case we can also suggest "... or list of these"

Since "of these" / "of that" is precisely what "thereof" means

It is correct english, but I have not seen it commonly used. For example:

ndarray of shape (n_samples, n_classes) or list of thereof

The thereof is referring to ndarray. I think this type of "going back through the sentence" and having to think about what thereof is referring will be difficult for some.

We can try it out and see if it leads to any confusion.

XREF: https://english.stackexchange.com/questions/27654/why-do-we-use-the-word-thereof

So let's try out. It is probably as confusing as the difference between an ndarray and an array-like at the difference that is documented in an English dictionary instead of our Glossary :)

@glemaitre could you apply the changes proposed in https://github.com/scikit-learn/scikit-learn/pull/18243/files#r475250655 ;)

Good point. It makes more sense.

adrinjalali · 2020-08-24T17:17:43Z

FWIW, I find list of such arrays much easier to parse than list of thereof. But happy with whatever y'all decide on.

sklearn/linear_model/_least_angle.py

doc/developers/contributing.rst

glemaitre · 2020-08-26T08:28:16Z

OK. So I added the different possibilities. An additional question since we are modifying now the guide, I want to state 2 things:

how to document when there is multiple dtype. I recall that we used a set array-like of shape (n_samples,), dtype={int, float}. Does it seem ok?
how to document the shape/length of a list. I can see some occurrence of list of shape (n_samples,). Do we want to keep this? Or do we want a specific way to state the shape of a list list of length n_samples? I think that the problem does not exist when we state array-like`.

ogrisel

I also find the thereof terminology not so clear (even if correct).

When documenting the length of the outer list of a nested list structure, I think it's important to document it explicitly as such. See example below, the other occurrences in this PR will need a similar update if your agree with this suggestion.

ogrisel · 2020-10-08T13:19:38Z

doc/developers/contributing.rst

-       ``ndarray of shape (n_samples,), dtype=np.int32``.
+       ``ndarray of shape (n_samples,), dtype=np.int32``. You can specify
+       multiple dtype as a set:
+       ``array-like of shape (n_samples,), dtype={int, float}``.


I would rather not recommend to use dtype=int in documentation. Either use a specific precision level (np.int32) or "integer dtype" or floating point dtype" if you want to mention arbitrary precision. Or just "numeric dtype" if both integer or floating point dtypes are accepted (but maybe dtype can be completely omitted in this case.

With numpy, passing dtype=int and dtype=float will result in np.int64 and np.float64 as actual dtypes but:

this is implicit;

this might be platform specific and could lead to subtle bugs (though I am not sure it's still the case with recent numpy versions and Python 3).

this might be platform specific and could lead to subtle bugs (though I am not sure it's still the case with recent numpy versions and Python 3).

I think this is still the case with the integer (or at least last week during a course :))

I made the correction avoiding using int and float and advise for using integer, floating and numeric.
I added some explanation in the glossary as well.

sklearn/linear_model/_least_angle.py

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

glemaitre · 2020-10-09T20:34:00Z

When documenting the length of the outer list of a nested list structure, I think it's important to document it explicitly as such. See example below, the other occurrences in this PR will need a similar update if your agree with this suggestion.

I update these cases as well.

ogrisel

Some more comment about the recommendation about the dtype doc recommendations.

doc/developers/contributing.rst

examples/model_selection/plot_learning_curve.py

ogrisel

LGTM

doc/developers/contributing.rst

Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

thomasjpfan · 2021-01-06T19:24:26Z

doc/developers/contributing.rst

+        * ``array-like of shape (n_samples,) or list thereof``;
+        * ``array-like of shape (n_samples,) or list of these``;
+        * ``array-like of shape (n_samples,) or list of such arrays``.


How does one choose between these three options? Personally, I would always go with list of such ___

of such ___, it is. Let's move forward with this.

…n#18243) Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

DOC mention 'of thereof' in doc guideline

e293dbf

github-actions bot added the Documentation label Aug 23, 2020

alfaro96 approved these changes Aug 23, 2020

View reviewed changes

doc/developers/contributing.rst Outdated Show resolved Hide resolved

Update contributing.rst

1a36644

thomasjpfan reviewed Aug 23, 2020

View reviewed changes

This was referenced Aug 24, 2020

DOC Add None as valid option to random_state for dummy.py #18245

Merged

DOC Add None as valid option to random_state in random_projection.py #18238

Merged

glemaitre added 3 commits August 25, 2020 10:39

Merge remote-tracking branch 'origin/master' into doc_thereof

d5459ca

Advise using thereof only for complex arrays

abbc760

Merge remote-tracking branch 'glemaitre/doc_thereof' into doc_thereof

e50cf2d

glemaitre changed the title ~~DOC mention 'of thereof' in doc guideline~~ DOC mention 'thereof' in doc guideline Aug 25, 2020

github-actions bot added the module:linear_model label Aug 25, 2020

thomasjpfan reviewed Aug 25, 2020

View reviewed changes

sklearn/linear_model/_least_angle.py Outdated Show resolved Hide resolved

NicolasHug reviewed Aug 25, 2020

View reviewed changes

doc/developers/contributing.rst Outdated Show resolved Hide resolved

iter

e1ebe2f

glemaitre changed the title ~~DOC mention 'thereof' in doc guideline~~ DOC update documentation guideline regarding docstring formatting Aug 26, 2020

marenwestermann added a commit to marenwestermann/scikit-learn that referenced this pull request Aug 27, 2020

minor docstring changes for consistency with scikit-learn#18243

65a8a30

alfaro96 mentioned this pull request Aug 28, 2020

Multiple dtype values example #18287

Closed

Merge branch 'master' into doc_thereof

7c2f418

glemaitre added the Waiting for Reviewer label Oct 5, 2020

ogrisel reviewed Oct 8, 2020

View reviewed changes

glemaitre and others added 2 commits October 9, 2020 20:58

Apply suggestions from code review

8031aa2

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

iter

3d7324c

ogrisel reviewed Oct 14, 2020

View reviewed changes

doc/developers/contributing.rst Outdated Show resolved Hide resolved

examples/model_selection/plot_learning_curve.py Outdated Show resolved Hide resolved

glemaitre added 3 commits October 19, 2020 10:54

solve conflict + review

02988f9

fix indent

478427f

iter

9cc3cb8

Merge remote-tracking branch 'origin/master' into doc_thereof

99cdfd3

ogrisel approved these changes Jan 6, 2021

View reviewed changes

jeremiedbb reviewed Jan 6, 2021

View reviewed changes

doc/developers/contributing.rst Outdated Show resolved Hide resolved

Update doc/developers/contributing.rst

b88ab24

Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>

thomasjpfan reviewed Jan 6, 2021

View reviewed changes

such arrays it is

0fe92eb

thomasjpfan approved these changes Jan 8, 2021

View reviewed changes

thomasjpfan changed the title ~~DOC update documentation guideline regarding docstring formatting~~ DOC Update docs guideline regarding docstring formatting Jan 8, 2021

thomasjpfan merged commit 266a11b into scikit-learn:master Jan 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC Update docs guideline regarding docstring formatting #18243

DOC Update docs guideline regarding docstring formatting #18243

glemaitre commented Aug 23, 2020

alfaro96 left a comment

thomasjpfan Aug 23, 2020

glemaitre Aug 23, 2020

alfaro96 Aug 23, 2020

NicolasHug Aug 23, 2020 •

edited

Loading

thomasjpfan Aug 23, 2020

NicolasHug Aug 23, 2020

thomasjpfan Aug 23, 2020

glemaitre Aug 24, 2020

NicolasHug Aug 24, 2020

glemaitre Aug 25, 2020

adrinjalali commented Aug 24, 2020

glemaitre commented Aug 26, 2020

ogrisel left a comment

ogrisel Oct 8, 2020

ogrisel Oct 8, 2020

glemaitre Oct 9, 2020

glemaitre Oct 9, 2020

glemaitre commented Oct 9, 2020

ogrisel left a comment

ogrisel left a comment

thomasjpfan Jan 6, 2021

glemaitre Jan 7, 2021

DOC Update docs guideline regarding docstring formatting #18243

DOC Update docs guideline regarding docstring formatting #18243

Conversation

glemaitre commented Aug 23, 2020

alfaro96 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NicolasHug Aug 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrinjalali commented Aug 24, 2020

glemaitre commented Aug 26, 2020

ogrisel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glemaitre commented Oct 9, 2020

ogrisel left a comment

Choose a reason for hiding this comment

ogrisel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NicolasHug Aug 23, 2020 •

edited

Loading