-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
More cleanup defaults in docstrings #15822
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
Conversation
@@ -1017,9 +1013,6 @@ def subplots(nrows=1, ncols=1, sharex=False, sharey=False, squeeze=True, | |||
always a 2D array containing Axes instances, even if it ends up | |||
being 1x1. | |||
|
|||
num : int or str, optional, default: None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is implicitly included in **fig_kw
and not worth mentioning explicitly here.
76eeac8
to
67d9d1e
Compare
@@ -306,7 +306,7 @@ def set_xlabel(self, xlabel, fontdict=None, labelpad=None, **kwargs): | |||
xlabel : str | |||
The label text. | |||
|
|||
labelpad : scalar, optional, default: None | |||
labelpad : float, default: ``self.xaxis.labelpad`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to decide how to document the "None means something" pattern which occurs quite frequently throughout the codebase. Do we mention explicitly the ability to pass None ("float or None"), and do we say "default: None" and later explain in the text that None means something something?
I think your approach (don't mention None and directly give the "effective" default) is much more practical, but this should be codified to avoid going back and forth with edits...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would only document default: None
explicitly if it has a dedicated meaning in the sense that it makes sense to pass func(param=None)
. In cases that just use None as a sentinel value for "not passed" I would not mention that in the docs. In these cases the sentinel value is not of interest to the user.
Regardless of that it's API and cannot be changed, but we don't have to advertise it in the semantic description of the docstring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think I have somewhere a patch for making a decorator that deprecates these "None as default" in the cases where the default can just as easily be spelled out (typically 0 or 1 or the empty string)... anyways that's a plan for another time.
67d9d1e
to
0541097
Compare
0541097
to
576614a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modulo ci and one capitalization nit.
576614a
to
351b10d
Compare
PR Summary
Continuation of #15818.