-
Notifications
You must be signed in to change notification settings - Fork 55
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
Constructor documentation duplication #436
Comments
Yea this (and related topics) is something we've discussed several times. The Google style guide suggests that the parameters should only be described in I am generally for removing duplicated documentation - it greatly increases our ability to maintain the code. Basically any time we find ourselves copy-pasting docstrings, I would rather do something else. One thing I've been playing around with in other projects is using |
I also remember @rileyjmurray talking about docstring templates at some point, but can't remember the exact context. |
I'm also fine with removing duplicated documentation. In this case I think that straight-up deleting the class docstrings and keeping in Responding to Stefan's last comment, here's a super simple decorator definition that can programmatically populate docstrings: def set_docstring(docstr):
def assign(fn):
fn.__doc__ = docstr
return fn
return assign Here's how I used it in other projects. The idea was to define an interface class that included a template docstring with a few blank spots, then subclasses would define a tuple of strings called that held content to substitute into the template. class TaskInterface:
"""
Class for doing the useful thing.
"""
TEMPLATE_DOCSTR = """
Given x and y, approximately compute
the_thing(x, y).
%s
Parameters
------------
x : something
y : something
tol : float
%s
Returns
--------
val : something
%s
"""
# Dummy values
INTERFACE_FIELDS = (
"""
Use [the method].
""",
"""
We use an iterative method, and stop once [metric] falls below tol.
""",
"""
Note, "val" is guaranteed to satisfy [condition].
"""
)
DOCSTR = TEMPLATE_DOCSTR % INTERFACE_FIELDS
@set_docstring(DOCSTR)
def __call__(x, y, tol):
raise NotImplementedError() |
I am going to preface this by acknowledging that there are many different perspectives on this, but wanted to open up a discussion to establish what our 'house style' ought to be.
See below an example of one of the classes from
workspaceplots.py
:The thing I would like to draw attention to is that the docstring for the class nearly exactly mirrors that if the
__init__
. Not all style guides agree here regarding whether the constructor documentation should be associated with the class or with the__init__
(i.e. there is a principled reason why some folks opt to have both serving slightly different purposes), but I'm personally partial to the perspective that you should pick one or the other, but not both (and I am agnostic as to which).TLDR: There is a lot of duplicated documentation here and I think we should prune it, both to cut down the length of the file, but also to reduce the number of places where we can make a mistake and fail to edit both instances of the docstrings when making a change. Thoughts?
The text was updated successfully, but these errors were encountered: