document default values as part of the type spec

[keewis]

Not sure if there is a better way to state that the colon is a matter of taste and has no effect on the generated documentation.

• closes syntax to document default values #284

[codecov-commenter]

Codecov Report

Merging #289 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #289   +/-   ##
=======================================
  Coverage   93.10%   93.10%           
=======================================
  Files           7        7           
  Lines        1261     1261           
=======================================
  Hits         1174     1174           
  Misses         87       87           

[jnothman]

I think then we need some advice on when "optional" is preferred over a default value. For me, "optional" should be used where the meaning of the default value would not be clear if written in place. For example None is often used as a placeholder for "disable some functionality" or "determine automatically", and this is better off described below the type spec.

We have adopted default= rather than default: in Scikit-learn. See https://scikit-learn.org/dev/developers/contributing.html#guidelines-for-writing-documentation

[keewis]

I think then we need some advice on when "optional" is preferred over a default value

sure, I'll try to add that.

We have adopted default= rather than default: in Scikit-learn

Ugh. Yes, I only ever looked at pandas and xarray when suggesting those. I imagine there are lots of projects with other formats, e.g. int (default None). Should we allow all of those, or would it be better to list a few options and discourage everything else (which might be easier to support)?

[jnothman]

I'm really not sure how to get community consensus. Saying that it is not currently standardised might be fair for now??

…

On Fri, 31 Jul 2020, 1:26 am keewis, ***@***.***> wrote: I think then we need some advice on when "optional" is preferred over a default value sure, I'll try to add that. We have adopted default= rather than default: in Scikit-learn Ugh. Yes, I only ever looked at pandas and xarray when suggesting those. I imagine there are lots of projects with other formats, e.g. int (default None). Should we allow all of those, or would it be better to list a few options and discourage everything else (which might be easier to support)? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#289 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAATH2ZGC5CI4HTPEZIAEVLR6GGMDANCNFSM4PLV32AA> .

[keewis]

I guess for now we could pick the three already mentioned (default , default: , and default=) and state that there might be more?

[jnothman]

I'd rather just display one that seems clear as an example.

[keewis]

the issue with that is that with just a single example we'd have to describe how other notations are allowed to be different, which would deviate from the current style of the guide.

For my original suggestion, how about:

or as part of the type, instead of ``optional``. If the default value would not be
used as a value, ``optional`` is preferred. These are all equivalent::

    copy : bool, default True
    copy : bool, default=True
    copy : bool, default: True

[rgommers]

For my original suggestion, how about:

That makes sense to me. I propose to do that given that that's the state we're in today, and it's better than not merging this PR.

[keewis]

done. @rgommers, was that what you were thinking of?

[rgommers]

Indeed, thanks @keewis, merged. Thanks @jnothman for reviewing too.