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

sphinx 3.2.0: trouble with typing.get_type_hints #8099

Closed
tyralla opened this issue Aug 11, 2020 · 3 comments
Closed

sphinx 3.2.0: trouble with typing.get_type_hints #8099

tyralla opened this issue Aug 11, 2020 · 3 comments

Comments

@tyralla
Copy link

tyralla commented Aug 11, 2020

Since 3.2.0, I cannot apply sphinx on my project (neither on Travis-CI nor locally):

Exception occurred:
  File "<string>", line 1, in <module>
NameError: name 'typingtools' is not defined

typingtools is module only required for typing purposes. Hence, I import it as follows:

if TYPE_CHECKING:
    from hydpy.core import typingtools

The problem starts in method AttributeDocumenter.add_directive_header() in line annotations = get_type_hints(self.parent).

If I apply the following manually, I get the same error:

import typing
from hydpy.models.arma.arma_control import Responses
typing.get_type_hints(Responses)

When I add module typingtools to the local variables, everything seems to work:

import typing
from hydpy.core import typingtools
from hydpy.models.arma.arma_control import Responses
typing.get_type_hints(Responses, localns={'typingtools': typingtools})

My project: https://github.com/hydpy-dev/hydpy/tree/develop.

@tk0miya tk0miya added this to the 3.2.1 milestone Aug 12, 2020
tk0miya added a commit to tk0miya/sphinx that referenced this issue Aug 12, 2020
…PE_CHECKING

`typing.get_type_hints()` raises NameError when the target object
contains unresolavable type annotation (ex. TYPE_CHECKING).  This
handles the exception and use unresolved annotations for type hints.
@tk0miya
Copy link
Member

tk0miya commented Aug 12, 2020

Sorry for the inconvenience. I posted #8108 to fix your error. Could you confirm it please?

tk0miya added a commit to tk0miya/sphinx that referenced this issue Aug 12, 2020
…PE_CHECKING

`typing.get_type_hints()` raises NameError when the target object
contains unresolavable type annotation (ex. TYPE_CHECKING).  This
handles the exception and use unresolved annotations for type hints.
tk0miya added a commit to tk0miya/sphinx that referenced this issue Aug 13, 2020
…PE_CHECKING

`typing.get_type_hints()` raises NameError when the target object
contains unresolavable type annotation (ex. TYPE_CHECKING).  This
handles the exception and use unresolved annotations for type hints.
@tyralla
Copy link
Author

tyralla commented Aug 13, 2020

Thank you very much for your quick help. Due to commit 611fff9 sphinx generates the documentation of hydpy without any errors again.

It is great that Sphinx now understands annotations including those written as forward references. However, due to commit 611fff9 just falling back to the standard __annotations__ whenever a NameError occurs, it misses all forward references imported within TYPE_CHECKING blocks.

Maybe it is more a question for enhancement than for a bugfix, but is there already a recommended way to give autodoc some general typing information to solve this shortcoming?

In hydpy, we generally import complete modules (and not only the required members) without changing their names. Hence, all we need is to pass all relevant modules and their names to get_type_hints via the localns argument.

To check that it principally works, I added the following simple get_localns function to autodoc.__init__, which collects all modules of the relevant sub-packages:

import os
from hydpy import core, auxs
def get_localns():
    localns = {}
    for package in [core, auxs]:
        for filename in os.listdir(package.__path__[0]):
            if not filename.startswith('_'):
                modulename = filename.split('.')[0]
                localns[modulename] = importlib.import_module(
                    f'{package.__name__}.{modulename}'
                )
    return localns

When I use get_localns, I don't require the NameError exception handling anymore and Sphinx understands all forward references perfectly:

            try:
                annotations = get_type_hints(
                    self.parent,
                    localns=get_localns()
                )
            # except NameError:
            #     # Failed to evaluate ForwardRef (maybe TYPE_CHECKING)
            #     annotations = safe_getattr(self.parent, '__annotations__', {})
            except TypeError:
                annotations = {}

So, at least for us, accepting a callback function would be sufficient.

@tk0miya
Copy link
Member

tk0miya commented Aug 14, 2020

Thank you for your confirmation! I'll merge and release it soon.

About TYPE_CHECKING, I'm also thinking to pass a dict to get_type_hints(). #8007 is one of my ideas to realize it. I feel it is annoying to me to define a package list twice in both source-code (TYPE_CHECKING block) and conf.py. So it would be wonderful if we can built them without any settings in the future! I'm still investigating the way to do that :-)

tk0miya added a commit that referenced this issue Aug 14, 2020
Fix #8099: autodoc: NameError is raised when script uses TYPE_CHECKING
@tk0miya tk0miya closed this as completed Aug 14, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants