Decorator @modify_docstring #943
Conversation
Codecov Report
@@ Coverage Diff @@
## master #943 +/- ##
==========================================
+ Coverage 96.19% 96.21% +0.02%
==========================================
Files 60 60
Lines 5413 5443 +30
==========================================
+ Hits 5207 5237 +30
Misses 206 206
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I do very much like this decorator! As we talked about earlier, we can wait until a future pull request to expand the functionality for this decorator...for example to inject the descriptions of common arguments like B that we keep having to repeat between functions.
The only substantive comment I have is to break out the different tasks in the decorator into private functions...since this will make it easier to work with the decorator later on should we expand its functionality.
Also...the file name test_helpers.py has the potential for ambiguity with the test helpers that I'm working on starting in #919. I'm wondering if we could change names to avoid this overlap. It's not important, but it did cause me a moment of confusion.
Thank you for doing this!
| -------- | ||
|
|
||
| >>> @modify_docstring(prepend='''Hello''', append='''World''') | ||
| ... def foo(): |
There was a problem hiding this comment.
| ... def foo(): | |
| ... def vegan_spam(): |
There was a problem hiding this comment.
This is staying foo. 😁
| if func is not None: | ||
| # `modify_docstring` called as a function | ||
| return decorator(func) | ||
| else: | ||
| # `modify_docstring` called as a decorator "sugar-syntax" | ||
| return decorator |
There was a problem hiding this comment.
This comes up for a whole bunch of decorators that have optional arguments, so I'm wondering if we should create a function for it. Doesn't have to be now, though.
There was a problem hiding this comment.
Maybe for the future. I do always look up one of our old decorators to remind me how to do it.
| # save the original docstring | ||
| setattr(wrapper, "__original_doc__", wrapper.__doc__) | ||
| doclines = inspect.cleandoc(wrapper.__doc__).splitlines() | ||
|
|
||
| # prepend docstring lines | ||
| if isinstance(prepend, str): | ||
| prependlines = inspect.cleandoc(prepend).splitlines() | ||
| prependlines.append("") | ||
| elif prepend is None: | ||
| prependlines = [] | ||
| else: | ||
| raise TypeError( | ||
| f"Expected type str for argument 'prepend', got {type(prepend)}." | ||
| ) | ||
|
|
||
| # append docstring lines | ||
| if isinstance(append, str): | ||
| appendlines = inspect.cleandoc(append).splitlines() | ||
| appendlines = [""] + appendlines | ||
| elif append is None: | ||
| appendlines = [] | ||
| else: | ||
| raise TypeError( | ||
| f"Expected type str for argument 'append', got {type(append)}." | ||
| ) | ||
|
|
||
| # define new docstring | ||
| wrapper.__doc__ = "\n".join(prependlines + doclines + appendlines) |
There was a problem hiding this comment.
This decorator has the potential to get significantly more complex, so I would suggest break up the prepending and appending (prependation and appendation?) into different private functions that can be called inside the decorator.
It could be something vaguely along the lines of:
doclines = _save_original_docstring(...)
prependlines = _prepend_docstring_lines(prepend)
appendlines = _append_docstring_lines(append)
wrapper.__doc__ = _define_new_docstring(prependlines, appendlines, doclines)where the comments describing the section of code become the function names (though there's probably a better way to break it up than the above code block). I think of this pattern of refactoring when there are sections of code separated by comments. I think I originally read it in Clean Code by R. Martin. We could also use the strategy of dropping one level of abstraction per function.
I'm also mentioning this because if I had followed this advice with the original implementation of plasmapy.utils.pytest_helpers.run_test, I wouldn't have needed to go through with the test helper refactoring that I'm doing in #919 through #929...oof.
There was a problem hiding this comment.
Yeah, I do have several ideas to tackle more complex scenarios. I'll probably migrate this to a class style decorator, much like we do for validate_quantities and ValidateQuantites. Once there, I'll have functionality for injecting into specific numpydoc sections.
However, I want to leave that for the future. I want to get this through so I can get PR #908 merged and move on with the diagnostics. That is much more important to me at this point.
There was a problem hiding this comment.
Sure! I'm happy to save that for the future, in particular if we create an issue or two on GitHub so that we can keep track of it for the future.
If we rename |
This is a simple decorator that allows for the programmatic prepending and/or appending of docstrings. The original docstring is stored in a new
__original_doc__attribute so it can still be retrieved.This is useful in a scenario where you are overriding a method and the signature does not change, but you want add a little more info to the original docstring.