Repository navigation
Sync-up docstrings in C version of the the decimal module #71966
Description
Activity
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).
skrah commented
on Aug 16, 2016 skrahmannequinMannequinI 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.
skrah commented
on Sep 2, 2016 skrahmannequinMannequin"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:
"is_infinite($self, /)\n--\n\n\"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.
skrah commented
on Sep 5, 2016 skrahmannequinMannequinSorry, Raymond, this is my code area. I said I'll review a patch.
24 remaining items
cc @skirpichev
Sorry, currently open pr seems to be the way to nowhere. We can't update docstrings manually. It's a waste of time, unless different implementations actually have different API/behaviour.
This should be also a relatively common task: we have policy (PEP 399) to provide pure-Python alternatives for accelerated modules like the decimal.
So, Raymond's proposal seems to be an option. Alternatively, we could extend the AC to inherit docstrings from the pure-Python module. Something like this (will copy signature and docstring):
/*[clinic input] _decimal.Context.__init__ = _pydecimal.Context.__init__ [clinic start generated code]*/ static int context_init_impl(PyObject *self, PyObject *prec, PyObject *rounding, PyObject *emin, PyObject *emax, PyObject *capitals, PyObject *clamp, PyObject *status, PyObject *traps) /*[clinic end generated code: output=8bfdc59fbe862f44 input=45c704b93cd02959]*/ { return context_setattrs( self, prec, rounding, emin, emax, capitals, clamp, status, traps ); }
Alternatively, we could extend the AC to inherit docstrings from the pure-Python module. Something like this (will copy signature and docstring):
+1 That would be of benefit for several modules that have C accelerators for pure python code.
For
decimal, the docstring copy will be super helpful. However, the signatures are more challenging because the two modules don't always agree (in ways that users typically don't care about). For example,logical_oruses different parameter names and positional only restrictions.Side note: PEP 399 is mostly aspirational. The C code and Pure Python code are almost never "identical" but should be made as close as reasonably possible (for example, the pure python version of
OrderedDictis the canonical version; however, unlike the C version, it will lose its coherency in concurrent code if called without locks).Reacted by Adam Turner, Stan Ulbrych and Sergey B KirpichevThis is not so easy to do, because Python that executes the Argument Clinic code needs to import or parse Python module from other Python version, which can contain syntax not supported in older versions.
Crazy idea for future: split the pyc file on two parts -- docstrings will be in a separate file that maps a qualname (and an index, because there may be several objects with the same qualname) to docstrings, and the code objects in the pyc file should only contain references. Then the docstrings could be read at runtime and reused by the C implementation. There may be even several locale specific docstring files. Of couse it needs changes at multiple levels, so I do not see this on horizon.
the signatures are more challenging because the two modules don't always agree (in ways that users typically don't care about). For example, logical_or uses different parameter names and positional only restrictions.
Of course, there can be reasons to have different interfaces, but for for
_decimalvs_pydecimalis looks more of less a historical accident.In #137697 these differences are fixed (with preference for C versions), except for dunder methods with extra
contextkwargs. I'm planning to resolve this as Serhiy suggested. BTW, renaming for positional-only parameters, probably, must be supported like that:/*[clinic input] _decimal.Context.add = _pydecimal.Context.add with a as x with b as y [clinic start generated code]*/Argument naming doesn't matter for positional-only parameters and I don't consider this to be a difference. In fact, probably the inspect.Signature must have a method to compare signatures, ignoring naming differences in positional-only parameters (something like
dummy_eq()in SymPy). Or just__eq__()can be adjusted.the pure python version of OrderedDict is the canonical version; however, unlike the C version, it will lose its coherency in concurrent code if called without locks
I think such implementation differences can be documented without need for separate docstrings.
the Argument Clinic code needs to import or parse Python module from other Python version, which can contain syntax not supported in older versions.
I'm not sure how severe it is.
New syntax, probably, will be more likely in newly added modules. If someone will propose a patch "lets just replace this code in _pydecimal.py with using walrus operator" - it will be rejected, right?
In this regard importing docstrings from the C extension looks simpler, but I would more like if docs will be near the pure-Python code (which will serve as poor mans replacement for unclear docs).
I would more like if docs will be near the pure-Python code (which will serve as poor mans replacement for unclear docs).
IMO that really understates their value. The examples come from the underlying decimal arithmetic specifcation and ARE a part of the documentation. I put A LOT of time and craftsmanship in to making them useful (many aspects of the decimal spec and its API are not obvious or intuitive). The person who wrote the C code largely wasn't interested in documentation at all.
By "pure-Python code" I meant actual implementation of the module (say
_pydecimal), not examples in docs. When documentation is poor or unclear - the module code is itself a documentation.That's why I think that docstrings should be nearby of the pure-Python implementation, not with C "equivalents". The C code is much more verbose, it's not available for introspection in runtime.
@erlend-aasland, what do you think on above idea? I propose new AC "decorator" like
@copy_docto alter default behavior of cloning existing functions. How severe looks for you Serhiy's objection about incompatible Python syntax across versions? The AC seems to be an internal thing for CPython and it's not clear for me how much it should be backward-compatible.Just copying docstrings from the pure-Python to the C module version (and, perhaps, in the opposite direction in few cases) now is much more simple, thanks to AC. But it will be an ethernal maintenance burden to keep them in sync. I would prefer to keep just a single copy of docs somewhere (in C or pure-Python versions), with a possibility to override this for concrete functions/methods.
Metadata
Metadata
Assignees
Labels
Projects
- Status
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