Repository navigation
Docstrings for every public name, and clearer messages for two mistakes - #305
Merged
Merged
Conversation
The docs site will get an API reference built from the docstrings. So every public function and class now has a full one, with arguments, return values, exceptions, warnings and a tested example. Success, Error and the three async dispatch functions had none. The module docstrings that named a function that doesn't exist, or showed a method returning a plain value, are fixed. A method that returns a plain value, as 4.x methods did, now logs one line. It says to return Success(value) or Error(code, message), and links to the migration guide. Before, the log showed a traceback into jsonrpcserver and the client got a bare Internal error, so there was nothing to go on. The response is the same as before. With debug=True its data has the same hint. serve() now says where it's listening when it starts. With the default host it says that it accepts connections on every network interface. The line goes to the jsonrpcserver.server logger, and to stderr when logging isn't configured. It still writes nothing when sys.stderr is None (#269). tests/test_docstrings.py runs the docstring examples and checks that every name in __all__ has a docstring.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This is the code half of the docs review from 2026-10-06. The docs half
follows in a second pull request, which builds the API reference from these
docstrings.
Docstrings. Every name in
__all__now has a Google-style docstring withits arguments, return value, exceptions and warnings. Most also have an
example, which
tests/test_docstrings.pyruns.Success,Errorand thethree
async_dispatch*functions had no docstring at all. The public names injsonrpcserver.response,jsonrpcserver.result,jsonrpcserver.codes,NODATAandglobal_methodshave one too. Some module docstrings were wrong:one named
dispatch_to_responses, which doesn't exist, and one showed a methodreturning a plain value. Those are fixed. No signatures changed.
A clearer log line for a plain return value. A 4.x method such as
return "pong"gives a bare Internal error on 5.x. On 5.0.9 the client atleast saw "did not return a valid Result" in
data. 5.0.10 rightly removedthat, so the only clue left was a traceback that pointed into jsonrpcserver.
Now the log says:
The response is unchanged. With
debug=True, itsdatakeeps the old textand adds the same hint. The new exception is a subclass of
AssertionError,so nothing that caught the old one breaks.
serve() says where it's listening. With the default host it warns that it
accepts connections on every network interface. It also says how to limit it
to localhost. The line is logged on
jsonrpcserver.server, as before. Whenlogging isn't configured it also goes to stderr, because the INFO log line went
nowhere and the server printed nothing at all. It skips stderr when it's
None, so the PyInstaller fix from #269 still holds.Checks run locally on Python 3.13: pytest with 100% coverage (251 tests), ruff
check, ruff format, mypy and pyright.