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

Resolving multiple imports guarded by typing.TYPE_CHECKING #375

Closed
lchojnacki opened this issue Jul 27, 2023 · 9 comments · Fixed by #393
Closed

Resolving multiple imports guarded by typing.TYPE_CHECKING #375

lchojnacki opened this issue Jul 27, 2023 · 9 comments · Fixed by #393

Comments

@lchojnacki
Copy link

Hello!

At the beginning: I saw #22, but this case looks like a separate issue.

Description

Version 1.15.0 introduced a change in the way imports guarded by if typing.TYPE_CHECKING are handled. The _resolve_type_guarded_imports function searches for TYPE_CHECKING occurrences using a regular expression, and then runs the code inside that block with exec.

This solution is pretty smart and it works, until we have multiple levels of TYPE_CHECKING guards.

Example

bokeh/model/model.py: https://github.com/bokeh/bokeh/blob/branch-3.3/src/bokeh/model/model.py#L42

if TYPE_CHECKING:
    from ..core.has_props import Setter

bokeh/core/has_props.py: https://github.com/bokeh/bokeh/blob/branch-3.3/src/bokeh/core/has_props.py#L98

if TYPE_CHECKING:
    from ..client.session import ClientSession
    from ..server.session import ServerSession

if TYPE_CHECKING:
    Setter = Union[ClientSession, ServerSession]

The resolver finds TYPE_CHECKING guard in model.py and tries to execute the import, but it's unable to find the Setter in has_props.py (the TYPE_CHECKING flag is set to False, and the file has_props.py is just being executed by python, not parsed by _resolve_type_guarded_imports function).

Proposition

I'm wondering if it would be possible to resolve this type of imports recursively, using _resolve_type_guarded_imports? Or do you have any other ideas? I guess I have to stay with version <1.15.0 until this gets fixed.

@gaborbernat
Copy link
Member

Your proposition sounds ok, PR welcome 🤗

@hojo0590
Copy link

hojo0590 commented Sep 5, 2023

Could be AbstractSetIntStr from pydantic/_internal/_utils in pydantic 2.x another one?
_utils.py: https://github.com/pydantic/pydantic/blob/v2.2.2/pydantic/_internal/_utils.py#L22
which get's accessed in
main.py: https://github.com/pydantic/pydantic/blob/v2.2.2/pydantic/main.py#L43

at least I get the same 'WARNING: Failed guarded type import with ImportError("cannot import name 'AbstractSetIntStr'' that you mentioned

@lchojnacki
Copy link
Author

@hojo0590 yes, looking at the lines you've pasted, it looks like the same problem.

rra added a commit to lsst-sqre/safir that referenced this issue Sep 12, 2023
Pydantic v2 uses TYPE_CHECKING more aggressively in ways that
sphinx-autodoc-typehints currently doesn't cope with correctly.
See tox-dev/sphinx-autodoc-typehints#375.
rra added a commit to lsst-sqre/safir that referenced this issue Sep 14, 2023
Pydantic v2 uses TYPE_CHECKING more aggressively in ways that
sphinx-autodoc-typehints currently doesn't cope with correctly.
See tox-dev/sphinx-autodoc-typehints#375.
@barik94
Copy link

barik94 commented Oct 12, 2023

@hojo0590 did you find solution to resolve this problem "cannot import name 'AbstractSetIntStr' from 'pydantic._internal._utils'" ?

@Mr-Pepe
Copy link
Contributor

Mr-Pepe commented Oct 24, 2023

I have implemented a draft that should fix this issue: #393

@lchojnacki Could you please test if that fix works for your Bokeh use case?

However, the fix does not work for Pydantic because Pydantic uses AnyClassMethod = classmethod[Any, Any, Any] in one of its blocks guarded by if TYPE_CHECKING. That code can not be executed by Python because TypeError: 'type' object is not subscriptable. I couldn't find any documentation for generic arguments on classmethod, so I don't know what the Pydantic folks are trying to achieve there. I guess code guarded by if TYPE_CHECKING is not really meant for execution but does that mean that it can contain invalid Python code?

@hojo0590
Copy link

hojo0590 commented Oct 24, 2023

@Mr-Pepe this looks like valid Python 3.12 - see PEP-659
Haven't had a look if my guess could be correct and their dev enforces Python 3.12 already (for which the type guarding would make even more sense then) though.

@Mr-Pepe
Copy link
Contributor

Mr-Pepe commented Oct 26, 2023

@hojo0590 Running the following with Python 3.12

from typing import Any

AnyClassMethod = classmethod[Any, Any, Any]

yields

Traceback (most recent call last):
  File "tmp.py", line 3, in <module>
    AnyClassMethod = classmethod[Any, Any, Any]
                     ~~~~~~~~~~~^^^^^^^^^^^^^^^
TypeError: type 'classmethod' is not subscriptable

I couldn't really find any information on whether code in if TYPE_CHECKING actually has to be executable, so I opened this thread on discuss.python.org. @gaborbernat Do you know any specific documentation on the topic or did you just assume that you could execute the code when you wrote this extension?

@gaborbernat
Copy link
Member

Just hopped 😳hence why if it fails is silent error.

@hojo0590
Copy link

hojo0590 commented Oct 26, 2023

@Mr-Pepe sorry, I did not make it clear, that I haven't tried (Python 3.12) myself yet - thanks for trying it, I learned something

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

Successfully merging a pull request may close this issue.

5 participants