Fix incorrect fields in docstrings #1453
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The link in the return type was invalid, since `IO[bytes]` is not a single entity. As these tags will be redundant with the next major pydoctor release, I removed them rather than fixing the `@rtype`.
ILegacyLogObserver is the local name under which this interface is imported, but is not the actual class name.
The asterixes are not part of the parameter name itself, so they shouldn't be used in the docstring, unlike the `def` line. Future versions of pydoctor will warn about these asterixes.
Also correct the actual documented return type where a Python 2 `str` was still used instead of `bytes`.
In some cases a superfluous colon was preventing the name from being recognised as such, but in other cases the name was actually missing.
The docstring claimed that the method would both return -1 and raise an unspecified exception if the descriptor no longer has a valid file descriptor number associated with it. Some implementations within Twisted can return -1, while none of them raise an exception.
The code can no longer raise that exception, ever since commit e14cc81 ("Remove deprecated code that allowed passing non-bytes values.").
The mentioned functions from the `os` module don't document any particular exception types they might raise, but anything other than `OSError` seems unlikely.
The syntax of the `@raise` field requires an exception type and if we don't know the exception type because we're not the original raiser, `Exception` is the most specific we can be.
While the epytext parser can handle these cases gracefully, they are still syntactically incorrect.
These do not end up in the output. Future pydoctor versions will warn about them.
mthuurne
force-pushed
the
10021-mthuurne-fix-docstrings
branch
from
3c7d92e
to
9ca5eab
Compare
Epytext has a dedicated `@keyword` field for these. In the past, using `@param` worked just as well, but in the near future pydoctor will start warning about `@param` for parameters that aren't included in the function signature.
The signature of this method was changed in commit d55d2a0 ("Support encoding to new OpenSSH private key format"), but the docstring wasn't updated to match.
The parameters were made more explicit in commit 8f9ba2c ("Add type hints for twisted.internet.interfaces and twisted.internet.base"), but the docstring wasn't updated to match.
This method takes multiple domains, but was documented as taking only a single domain.
The documented `reactor` parameter does not exist, while the unrelated `threadFactory` parameter was undocumented.
mthuurne
force-pushed
the
10021-mthuurne-fix-docstrings
branch
from
9ca5eab
to
e975cf3
Compare
6 tasks
twm
approved these changes
Oct 21, 2020
There was a problem hiding this comment.
This looks good. I didn't run pydoctor, but all these changes are clearly improvements. I did note one typo.
The only other thought I had is that there are several places documented as Exception that might technically produce BaseException. However that's always true so 🤷♂️ .
Thank you!
| @@ -1588,9 +1588,6 @@ class IFileDescriptor(ILoggingContext): | |||
|
|
|||
| def fileno() -> object: | |||
| """ | |||
| @raise: If the descriptor no longer has a valid file descriptor | |||
| number associated with it. | |||
There was a problem hiding this comment.
It seems like a deficiency in the interface that there is no exception type defined for this. Oh well.
There was a problem hiding this comment.
None of the implementations within Twisted raise an exception though. Some return -1 if the descriptor is no longer valid.
I think the purpose of |
Co-authored-by: Tom Most <twm@freecog.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contributor Checklist:
tox -e black-reformatto format my patch to meet the Twisted Coding StandardI have updated the automated tests.reviewto the keywords field in Trac, and putting a link to this PR in the comment; it shows up in https://twisted.reviews/ now.