Skip to content
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

Imported string constant has wrong docstring of str #12020

Open
aeisenbarth opened this issue Feb 26, 2024 · 4 comments
Open

Imported string constant has wrong docstring of str #12020

aeisenbarth opened this issue Feb 26, 2024 · 4 comments

Comments

@aeisenbarth
Copy link

aeisenbarth commented Feb 26, 2024

Describe the bug

When a string constant is imported, sphinx cannot find the docstring of the original definition (in the module form where it was imported). Instead, it gives the docstring of str, which is misfitting and has a signature that does not render nicely.

image

Expected behavior

  • It should rather not show a description than showing a wrong (and ugly) description.
    The docstring of a string instance can't be the docstring of the str class.

Cause

getdoc_internal first tries the __doc__ attribute when looking for a docstring.

def getdoc_internal(

Demonstration:

>>> "some_string".__doc__
"str(object='') -> str\nstr(bytes_or_buffer[, encoding[, errors]]) -> str\n\nCreate a new string object from the given object. If encoding or\nerrors is specified, then the object must expose a data buffer\nthat will be decoded using the given encoding and error handler.\nOtherwise, returns the result of object.__str__() (if defined)\nor repr(object).\nencoding defaults to sys.getdefaultencoding().\nerrors defaults to 'strict'."

This is related to #6495

How to Reproduce

This is specifically a problem when using recursion with a custom template with members (documenting also object type data).
A less realistic but shorter way to trigger it is directly referencing the imported constant:

my_module/submodule1.py

MY_CONSTANT = "my_constant"
"""Docstring of MY_CONSTANT"""

my_module/submodule2.py

from my_module.submodule import MY_CONSTANT

index.rst

.. autosummary::

   my_module.submodule2.MY_CONSTANT

Environment Information

Platform:              linux; (Linux-5.15.0-91-generic-x86_64-with-glibc2.35)
Python version:        3.9.17 (main, Jul  5 2023, 20:41:20) 
[GCC 11.2.0])
Python implementation: CPython
Sphinx version:        7.2.6
Docutils version:      0.20.1
Jinja2 version:        3.1.2
Pygments version:      2.16.1

Sphinx extensions

["sphinx.ext.autodoc", "sphinx.ext.autosummary"]

Additional context

No response


Edit: I had forgotten to include the docstring I was using.

@electric-coder
Copy link

electric-coder commented Feb 27, 2024

Module level constants don't have docstrings, see How to document a module constant in Python?.

So the REPL you included is correct:

>>> my_var = "one_level"
>>> my_var.__doc__
"str(object='') -> str\nstr(bytes_or_buffer[, encoding[, errors]]) -> str\n\nCreate a new string object from the given object. If encoding or\nerrors is specified, then the object must expose a data buffer\nthat will be decoded using the given encoding and error handler.\nOtherwise, returns the result of object.__str__() (if defined)\nor repr(object).\nencoding defaults to sys.getdefaultencoding().\nerrors defaults to 'strict'."

sphinx cannot find the docstring of the original definition

You don't include an MRE does adding a docstring that Sphinx can parse as show in this post work?


Expected behavior

It should rather not show a description than showing a wrong

Apparently this exact issue can be worked around with this answer. I'm not sure how things work with autosummary but I'm in favor of having Sphinx fail explicitly over magically filtering the inherited docstring. Since there is a known workaround the argument in favor of magically filtering should include a reasoning of why it makes sense and why it's preferable over giving an explicit queue to the end user that they can put into google.

This question is a continuation of the much older #344 that also references this problem.

@aeisenbarth
Copy link
Author

Sorry, the example was not completely the code I was using and that I expected to work. I did have a docstring that was parsed by Sphinx for the original module, but obviously can't and doesn't get parsed when imported into another.


I agree that what is the right docstring and the chain of inheritance can be controversial, and complicating the rules with magical filtering will break as many use cases as it fixes. Still, for this example, I'm not sure if an instance without docstring should fall back to the class's docstring (especially a built-in basic type), since instances and classes are different things.

Although the str docstring might be correct according to the rules, it is of very little informative value.

As a user, I might have these options:

  • Put documentation to the right thing and avoid inheritance. This is what I did (in the updated example), but fails because docstrings seem not always to be propagated through imports.
  • Use workarounds like the one you linked (thanks), which documents the item with docstring in the original module, but displays instead the location where it has been imported. This requires manually adding a directive and option for this individual object. Unfortunately, when using autosummary with :recursive:, this workaround cannot be applied because recursion uses the same options for all objects.
  • The workaround I am using is to define __all__ so that it excludes the imports, and configure autosummary_ignore_module_all = False. The objects are then only documented at their original location with correct docstring. This way I don't need to control it through autodoc options in rst files, so that I can recursively generate them.

I also use already too many Sphinx workarounds, and instead of accumulating more, It would be great to find solutions at the source. I don't want to impose a solution, but it would be great if any of these were feasible:

  • Fix propagation of docstrings when importing constants. Improve autodata docstring detection #6495
  • Fix detection of whether an object has been imported (only working for module or class, not data; it is listed as direct member of this module and not in imported_members).
  • More control over what gets inherited and what not. Here, a user could configure it not to inherit from basic types (str). Also, when subclassing third-party classes with hundreds of methods, a user could maybe just document objects inherited from their own package.
  • More control over autodoc option overrides for individual objects when using generated rst files with :recursive:.

@picnixz
Copy link
Member

picnixz commented Mar 4, 2024

The issue is likely from the ModuleAnalyzer. I don't think we actually follow correctly the imports for that. Unfortunately this is not easy to fix so I can only add this issue to the tracker...

@smarie
Copy link

smarie commented May 20, 2024

Same error here.
Note that in the past it seems to have worked : #344

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants