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

typing.TYPE_CHECKING conditional imports break sphinx doc generation #22

Closed
henryJack opened this issue Sep 27, 2017 · 22 comments
Closed

Comments

@henryJack
Copy link

henryJack commented Sep 27, 2017

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
@agronholm
Copy link
Collaborator

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
Copy link
Collaborator

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

@henryJack
Copy link
Author

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

@agronholm
Copy link
Collaborator

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.

@henryJack
Copy link
Author

Good discussion here on how typing can lead to circular imports: python/typing#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 henryJack reopened this Sep 27, 2017
@agronholm
Copy link
Collaborator

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.

@henryJack
Copy link
Author

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
Copy link
Author

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

@agronholm
Copy link
Collaborator

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
Copy link
Collaborator

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

@henryJack
Copy link
Author

henryJack commented Sep 27, 2017

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

@agronholm
Copy link
Collaborator

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.

@henryJack
Copy link
Author

henryJack commented Sep 27, 2017

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
Copy link
Author

henryJack commented Sep 27, 2017

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?

@agronholm
Copy link
Collaborator

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

@agronholm
Copy link
Collaborator

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

@frankier
Copy link

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?

maroux referenced this issue in maroux/hedwig-python Oct 4, 2018
- Also add autodoc typing plugin to make typing hints documentation just slightly better. A side effect of agronholm/sphinx-autodoc-typehints#22 is that we have to remove typing hints for circular imports
facebook-github-bot referenced this issue in facebook/Ax Mar 11, 2019
Summary:
sphinx-autodoc-typehints (https://github.com/agronholm/sphinx-autodoc-typehints) isn't happy with importing specific classes within the conditional type checking statement (used to avoid circular dependencies). See https://github.com/agronholm/sphinx-autodoc-typehints/issues/22 for this issue.

The way to get around this, following https://github.com/agronholm/sphinx-autodoc-typehints#dealing-with-circular-imports and discussion on GitHub issue, is to import the module. I had to go two levels up often and import top-level modules in order for this to all work properly.

Reviewed By: ldworkin

Differential Revision: D14400529

fbshipit-source-id: 5cb726201072aa6084e2a918dadffefcd1b15afc
@adamcunnington-mlg
Copy link

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?

@gaborbernat
Copy link
Member

A PR for this would we welcome.

@agronholm
Copy link
Collaborator

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.

@alxroyer
Copy link

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.

@hoodmane
Copy link
Collaborator

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.

@gaborbernat gaborbernat closed this as not planned Won't fix, can't repro, duplicate, stale Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants