sphinx-autodoc-typehints icon indicating copy to clipboard operation
sphinx-autodoc-typehints copied to clipboard

typing.TYPE_CHECKING conditional imports break sphinx doc generation

Open henryJack opened this issue 8 years ago • 19 comments

Hi,

I've created a dummy repo highlighting my issue. There is a python module called mymodule and a README.md explaining my full step by step issue...

https://github.com/henryJack/sphinx_issue/blob/master/README.md

Problem

  • When using the sphinx-autodoc-typehints extension with python 3.5+ typings It fails to build. The error meesage is:
Exception occurred:
  File "<string>", line 1, in <module>
NameError: name 'B' is not defined
The full traceback has been saved in C:\Users\HENRY~1.TAN\AppData\Local\Temp\sphinx-err-wnuuyq83.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.

Procedure to reproduce the problem

Please see https://github.com/henryJack/sphinx_issue/blob/master/README.md

Basically, the problem lies in declaring imports using TYPE_CHECKING

from typing import TYPECHECKING
if TYPE_CHECKING:
    from mymodule import MyClass

class NewClass:
    def __init__(my_object `MyClass` = MyClass()):
        self.object: `MyClass` = myobject

Error logs / results

Exception occurred:
  File "<string>", line 1, in <module>
NameError: name 'B' is not defined
The full traceback has been saved in C:\Users\HENRY~1.TAN\AppData\Local\Temp\sphinx-err-wnuuyq83.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.

Expected results

It breaks the docs generator.

Environment info

  • OS: Windows 10
  • Python version: 3.6.1
  • Sphinx version: latest

henryJack avatar Sep 27 '17 13:09 henryJack

Aside from the fact that your example isn't valid Python syntax, what do you expect me to do? I could only think of setting typing.TYPECHECKING to True. Is that it?

agronholm avatar Sep 27 '17 14:09 agronholm

Your repository contains a circular import. I don't see how it could work even if TYPE_CHECKING was set to True.

agronholm avatar Sep 27 '17 14:09 agronholm

I have created a repository with a fully working example and step by step instructions on how to reproduce the error. https://github.com/henryJack/sphinx_issue/blob/master/README.md

The module in question is here https://github.com/henryJack/sphinx_issue/tree/master/mymodule. Yes, it does contain a circular import (which can happen when you introduce python 3.5 type hinting into the mix). The code is still valid python without it though

henryJack avatar Sep 27 '17 14:09 henryJack

But what do you expect me to do about this? As I just said, forcing typing.TYPECHECKING to True will just trigger that circular import and will cause sphinx to fail.

agronholm avatar Sep 27 '17 14:09 agronholm

Good discussion here on how typing can lead to circular imports: https://github.com/python/typing/issues/105...

The issue is that everything works up until the point you add the sphinx-autodoc-typehints extension to the conf.py file. If you have any ideas what the issue is here, I would love to have a go at fixing it myself.

henryJack avatar Sep 27 '17 14:09 henryJack

Well, fix the circular import. Then you won't even need the typing.TYPECHECKING condition. Running the code normally will not cause 'B' to be looked up so it won't trigger the error.

agronholm avatar Sep 27 '17 14:09 agronholm

It should be changed so that string type annotations should never get evaluated. Python runs this just fine...

from typing import TYPE_CHECKING if TYPE_CHECKING: from mymodule.class_b import B

class AClass: """ A Simple Class :param 'B' my_b: small amount of info

henryJack avatar Sep 27 '17 14:09 henryJack

Circular imports for type checking are valid, they don't need to be fixed

henryJack avatar Sep 27 '17 14:09 henryJack

You're talking about static typechecking here. Libraries that get type information at run time, like this extension, need to resolve them properly via imports.

agronholm avatar Sep 27 '17 14:09 agronholm

This is necessary in order to generate the links to the appropriate sections of the API documentation.

agronholm avatar Sep 27 '17 14:09 agronholm

Wouldn't it be better though if offending types are just ignored and displayed as strings and not links. Currently, it breaks the whole make docs process. It's a great library and I would still like to use it

henryJack avatar Sep 27 '17 14:09 henryJack

Yes, that would be an acceptable fix. I'm just wondering if it should generate some kind of warning too, so as not to hide any problems.

agronholm avatar Sep 27 '17 14:09 agronholm

Ideally, A warning + unlinked string would be great. This other example would also break the sphinx build and is 100% valid python

class Assembly:
    def __init__(self) -> None:
        self.sub_assemblies: List['Assembly'] = None

henryJack avatar Sep 27 '17 14:09 henryJack

As far I can tell, the code for sphinx-autodoc-typehints is just one file, right (sphinx_autodoc_typehints.py)? If you could just highlight which areas I would need to look at to intercept an error like this, I can have a crack at improving this?

henryJack avatar Sep 27 '17 15:09 henryJack

That last example would not break this extension because it only looks at function level annotations.

agronholm avatar Sep 27 '17 16:09 agronholm

Here is the code you'll want to look at.

agronholm avatar Sep 27 '17 16:09 agronholm

Firstly, thank you agronholm for your work on this module. The current PR seems like a reasonable first fix, and it would be nice to see it merged, but I had the beginnings of another idea. Might it be possible to perform two passes? On the first pass TYPE_CHECKING is False, and on the second True. No circular imports would occur on the second try since the relevant modules would have already executed all the way through and be in sys.modules. Thoughts?

frankier avatar Sep 20 '18 18:09 frankier

I have just ran into this issue - similar to the closed MR from MyST-Parser.

# Python version: 3.9.7 (CPython)
# Docutils version: 0.17.1 release
# Jinja2 version: 3.0.2
# Last messages:
#   making output directory...
#   done
#   myst v0.15.2: MdParserConfig(renderer='sphinx', commonmark_only=False, enable_extensions=['dollarmath'], dmath_allow_labels=True, dmath_allow_space=True, dmath_allow_digits=True, dmath_double_inline=False, update_mathjax=True, mathjax_classes='tex2jax_process|mathjax_process|math|output_area', disable_syntax=[], url_schemes=['http', 'https', 'mailto', 'ftp'], heading_anchors=None, heading_slug_func=None, html_meta=[], footnote_transition=True, substitutions=[], sub_delimiters=['{', '}'], words_per_minute=200)
#   [autosummary] generating autosummary for: api.md, index.md
#   building [mo]: targets for 0 po files that are out of date
#   building [html]: targets for 2 source files that are out of date
#   updating environment:
#   [new config]
#   2 added, 0 changed, 0 removed
#   reading sources... [ 50%] api
# Loaded extensions:
#   sphinx.ext.mathjax (4.2.0) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/ext/mathjax.py
#   sphinxcontrib.applehelp (1.0.2) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinxcontrib/applehelp/__init__.py
#   sphinxcontrib.devhelp (1.0.2) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinxcontrib/devhelp/__init__.py
#   sphinxcontrib.htmlhelp (2.0.0) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinxcontrib/htmlhelp/__init__.py
#   sphinxcontrib.serializinghtml (1.1.5) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinxcontrib/serializinghtml/__init__.py
#   sphinxcontrib.qthelp (1.0.3) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinxcontrib/qthelp/__init__.py
#   alabaster (0.7.12) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/alabaster/__init__.py
#   myst_parser (0.15.2) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/myst_parser/__init__.py
#   sphinx.ext.autodoc.preserve_defaults (1.0) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/ext/autodoc/preserve_defaults.py
#   sphinx.ext.autodoc.type_comment (4.2.0) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/ext/autodoc/type_comment.py
#   sphinx.ext.autodoc (4.2.0) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/ext/autodoc/__init__.py
#   sphinx.ext.autosummary (4.2.0) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/ext/autosummary/__init__.py
#   sphinx.ext.todo (4.2.0) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/ext/todo.py
#   sphinx.ext.viewcode (4.2.0) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/ext/viewcode.py
#   sphinx_autodoc_typehints (unknown version) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx_autodoc_typehints.py
#   sphinx_material (0.0.35) from /home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx_material/__init__.py
Traceback (most recent call last):
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/cmd/build.py", line 280, in build_main
    app.build(args.force_all, filenames)
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/application.py", line 343, in build
    self.builder.build_update()
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 293, in build_update
    self.build(to_build,
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 307, in build
    updated_docnames = set(self.read())
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 414, in read
    self._read_serial(docnames)
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 435, in _read_serial
    self.read_doc(docname)
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/builders/__init__.py", line 475, in read_doc
    doctree = read_doc(self.app, self.env, self.env.doc2path(docname))
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/io.py", line 189, in read_doc
    pub.publish()
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/docutils/core.py", line 217, in publish
    self.document = self.reader.read(self.source, self.parser,
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/sphinx/io.py", line 109, in read
    self.parse()
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/docutils/readers/__init__.py", line 78, in parse
    self.parser.parse(self.input, document)
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/myst_parser/sphinx_parser.py", line 53, in parse
    parser = default_parser(config)
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/myst_parser/main.py", line 139, in default_parser
    from myst_parser.sphinx_renderer import SphinxRenderer
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/myst_parser/sphinx_renderer.py", line 26, in <module>
    from myst_parser.docutils_renderer import DocutilsRenderer
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/myst_parser/docutils_renderer.py", line 41, in <module>
    from myst_parser.mocking import (
  File "/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/myst_parser/mocking.py", line 21, in <module>
    from .docutils_renderer import DocutilsRenderer
ImportError: cannot import name 'DocutilsRenderer' from partially initialized module 'myst_parser.docutils_renderer' (most likely due to a circular import) (/home/ac/projects/medialabgroup/apollo/bq-template-manager/.venv/lib/python3.9/site-packages/myst_parser/docutils_renderer.py)

When is this going to get fixed?

adamcunnington-mlg avatar Nov 02 '21 19:11 adamcunnington-mlg

A PR for this would we welcome.

gaborbernat avatar Jan 08 '22 11:01 gaborbernat

I recently bumped into this issue in other projects too. The problem here is that any _typeshed imports inevitably generate warnings, causing the build to fail when run with the -W flag. If there was some way to sweep these warnings under the carpet, that's what I would like to do.

agronholm avatar Apr 09 '23 16:04 agronholm

Firstly, thank you agronholm for your work on this module. The current PR seems like a reasonable first fix, and it would be nice to see it merged, but I had the beginnings of another idea. Might it be possible to perform two passes? On the first pass TYPE_CHECKING is False, and on the second True. No circular imports would occur on the second try since the relevant modules would have already executed all the way through and be in sys.modules. Thoughts?

Smart suggestion.

For the memo, I've given a try with it on my own project (I shall update this reply with a link to it later). It worked eventually, but I had to fix a couple of things around initializations in the library to make it possible.

In the end, I bet a better approach would be to switch to static analysis of the code (as mypy does) to extract documentation from it.

I just came to know about sphinx-extensions2/sphinx-autodoc2, which claims something around this topic. Not tried yet.

alxroyer avatar Apr 24 '23 05:04 alxroyer

It seems to me that the reproduction in the OP here was resolved by #201 (and perhaps #393 as well). Is there a remaining issue here? I think we should close this and open a new issue if there are remaining problems.

hoodmane avatar Nov 16 '23 20:11 hoodmane