-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
Sync-up docstrings in C version of the the decimal module #71966
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
Comments
The pure python version of decimal has extensive docstrings with text and explanatory examples drawn directly from the decimal arithmetic specification. Those should all be copied to the C version as well. This will improve usability for interactive use and improve help(). I would like to leave this work for one of the new aspiring core devs (perhaps Elizabeth, Lisa, or Nofar). |
I found the docstrings a bit too verbose (the power() docstring takes up more than a page), but I'm happy to add it and review a patch. The patch authors need to take the hand written function signatures into account. |
Historically, we've kept the docstrings 100% intact between the C and Python versions, even if they are big (see heapq.__doc__) for example. If there are differences in the "hand written" versions. I would like to have them compared on a case by case basis and resolved. Unless the C version implements something different, there should be no reason for a docstring difference. The originals are very close to the specification and to the documentation. We tried to stay true in part because there are so many niggling details that it would be hard to know without reading the spec. |
I will start working on the patch for this! Thanks for pointing me this way, Raymond. |
"hand written signatures" refers to the signatures that Argument Clinic would generate if it were used (but I don't want that). So this is an example of a hand written signature:
I still wonder if people wouldn't be served better with concise docstrings, but then I'm generally in favor of a Unix manual page documentation style, so perhaps there's a cultural difference. |
Sorry, Raymond, this is my code area. I said I'll review a patch. |
Lisa is not a committer. The assignment means that she is working on the patch. BTW, the decimal package has long been my area as well (writing a C implementation does not give you exclusive decision making over the docstrings.) |
Hi Stefan and Raymond, Here's my start on the patch, I wanted to get your opinions on the direction before I go too far. I've been comparing the two sets of docstrings, and trying to synchronize them in the clearest way that most closely matches the decimal specification docs. Let me know what you think! |
I just noticed some trailing whitespace in the patch, ignore them for now and they'll be removed in the next patch. |
Anyone get the chance to look over this yet? |
Raymond: "code area" meant literally that -- all code under Modules/_decimal/* is by myself. It is well understood that you and many people (e.g. Mark Dickinson) I want to know what is going into that code area. Lisa: I think I can take a look in the weekend. |
Lisa, thanks for the patch. I've left some comments -- some docstrings in the Python version are outdated, some not quite correct, some are not very clear (to me). I don't know how to proceed. Initially I thought it would be as easy as just taking over all Python docstrings verbatim, but looks like there's more work involved. |
Thanks for taking a look Stefan! I agree, it is definitely not as easy as it sounds. Your review and comments are helpful, I will make adjustments to the docstrings. If you want, I can continue to try to sync-up the docstrings and submit them for you and Raymond to review? I've been checking the docstrings against the general decimal specification: http://speleotrove.com/decimal/decarith.html, and with additional eyes on readability and best practices hopefully we can write updated, synchronized docstrings. |
Okay great. I think it's probably best to produce an initial patch with the verbatim Python docstrings (you can of course address the comments that I already made), then we mark the passages that are clearly not valid for _decimal or outdated for _pydecimal, then we go for extra clarity. |
This (should) be the patch with the python docstrings copied over to the C version. |
Patches are prepared but not continued. It can be merge by small additions to the patches. |
One way to do is to dynamically update the docstrings on import. Something like this: for name in dir(_decimal.Decimal):
if name.startswith('_'):
continue
py_method = getattr(_decimal.Decimal, name)
py_doc = py_method.__doc__
if py_doc is None:
continue
c_method = getattr(_pydecimal.Decimal, name, None)
if c_method is None:
continue
c_doc = c_method.__doc__
if c_doc is None or len(c_doc) < len(py_doc):
c_method.__doc__ = py_doc |
Now it is set even if the C implementation is used. Also add a one-line synopsis.
…7919) Now it is set even if the C implementation is used. Also add a one-line synopsis.
…ythonGH-117919) Now it is set even if the C implementation is used. Also add a one-line synopsis. (cherry picked from commit c69968f) Co-authored-by: Serhiy Storchaka <[email protected]>
…ythonGH-117919) Now it is set even if the C implementation is used. Also add a one-line synopsis.
…GH-117919) (GH-117962) Now it is set even if the C implementation is used. Also add a one-line synopsis. (cherry picked from commit c69968f)
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: