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

Import errors in a try/except ImportError block should be allowed #1393

Open
bdarnell opened this issue Apr 17, 2016 · 19 comments
Open

Import errors in a try/except ImportError block should be allowed #1393

bdarnell opened this issue Apr 17, 2016 · 19 comments

Comments

@bdarnell
Copy link
Contributor

When this pattern is checked in py2 mode (or really any pre-3.5 version)

try:
    from inspect import isawaitable  # py35+
except ImportError:
    from backports_abc import isawaitable

it raises an error tornado/gen.py:118: error: Module has no attribute 'isawaitable'. Instead, mypy should recognize that this is within a try/except block that catches the ImportError and choose the alternate path instead.

This falls under the umbrella of #1297 but I couldn't find an exact match for this issue.

@gvanrossum
Copy link
Member

gvanrossum commented Apr 17, 2016 via email

@bdarnell
Copy link
Contributor Author

Yes, # type: ignore works here (as long as I ignore both branches)

@gvanrossum gvanrossum added this to the 0.5 milestone Apr 28, 2016
@gvanrossum gvanrossum removed this from the 0.5 milestone Mar 29, 2017
@mig4
Copy link

mig4 commented Jan 3, 2019

This is also the case with attribute access in try/except block, not only imports, e.g.:

try:
    foo = sys._MEIPASS
except AttributeError:
    foo = '.'

causes mypy to fail with error: Module has no attribute "_MEIPASS", even though it looks like a reasonable approach. Ignoring with # type: ignore comment silences it.

@bhrutledge
Copy link

We encountered this in Twine with importlib.metadata.metadata vs importlib_metadata.metadata, but only as of mypy 0.750. There's a fix pending in pypa/twine#551 (which also covers an instance of #1153).

I made a repository to reproduce the errors and try out some fixes: https://github.com/bhrutledge/mypy-importlib-metadata. That's currently passing, but the commit history has more details.

@emmatyping
Copy link
Collaborator

The change is likely due to a change in typeshed (adding importlib_metadata I presume). I would recommend using if sys.version_info... checks to tell mypy what Python version is being used in each case so it doesn't misunderstand the code.

@bhrutledge
Copy link

@ethanhs Thanks! I didn't know that mypy would use sys.version_info as a hint.

@jaraco
Copy link
Member

jaraco commented Dec 3, 2019

I would recommend using if sys.version_info...

This approach is less desirable than using the try/except pattern. In particular, one must (a) import sys, (b) parse/compare sys.version_info, and (c) encode availability of module features to their various versions (which may not be a simple >= comparison).

Is the implication of this recommendation that try/except for imports is deprecated/discouraged, or is this recommendation just a temporary workaround for an acknowledged problem?

@JukkaL
Copy link
Collaborator

JukkaL commented Dec 4, 2019

A workaround is needed because of a missing mypy feature. I don't know when the feature might be implemented, though.

@The-Compiler
Copy link
Contributor

Perhaps a naive question: If mypy recognized a "try/except ImportError", could it set the type of the module object to something like Union[Module1, Module2] (or Optional[Module] in case the except does mod = None)?

@emmatyping
Copy link
Collaborator

emmatyping commented Dec 22, 2019

@The-Compiler sadly that probably wouldn't work, as you want to make sure the same module is used throughout. We would probably want something closer to a type variable I believe.

@ghost
Copy link

ghost commented Feb 23, 2022

I just ran into the same thing today with this:

try:
    from typing import TypeAlias
except ImportError:
    from typing_extensions import TypeAlias

Since I have this in my pyproject.toml (rather not add an extra dependency when not necessary):

[tool.poetry.dev-dependencies]
# ...
typing-extensions = {version = "^4.1.1", python = "<3.10"}

# type: ignore[attr-defined] works in the meantime.

@adam-grant-hendry
Copy link

adam-grant-hendry commented Jun 12, 2022

I've encountered this as well.

From Adam Johnson's blog post, his workaround is to set a boolean flag:

try:
    import markdown

    HAVE_MARKDOWN = True
except ImportError:
    HAVE_MARKDOWN = False


def something():
    ...

    if HAVE_MARKDOWN:
        ...

In fact, @Michael0x2a on this SO post couples the above approach with the --always-true/--always-false options to type check both implementations.

Alternatively, @cjerdonek has developed a nice workaround.

Perhaps a naive question: If mypy recognized a "try/except ImportError", could it set the type of the module object to something like Union[Module1, Module2] (or Optional[Module] in case the except does mod = None)?

@The-Compiler No, I don't think it's naive at all. In fact, I believe @JukkaL hinted at this. However, @ethanhs might be right: I believe (emphasis on "believe") the root of the issue is that mypy is "picky" in the sense that the module/package API must be consistent throughout.

@JukkaL Can you confirm?

Addendum

Below are gists of Adam Johnson's failed attempt and @cjerdonek's working workaround:

Failing Adam Johnson gist
Passing @cjerdonek gist

adamnovak added a commit to DataBiosphere/toil that referenced this issue Jul 17, 2023
MyPy can't handle a Pythonic try-except import: python/mypy#1393
We could put # type: ignore[attr-defined] except then whichever branch
actually passed on the currently installed version of the module would
fail due to having an apparently-unneeded ignore.
DailyDreaming pushed a commit to DataBiosphere/toil that referenced this issue Jul 18, 2023
* Use a vg-like system toa void needing a Kubernetes docker builder

* Specify inline caching correctly

* Drop extra import

* Fix the wandering typedef two ways

* Fix it only one way

MyPy can't handle a Pythonic try-except import: python/mypy#1393
We could put # type: ignore[attr-defined] except then whichever branch
actually passed on the currently installed version of the module would
fail due to having an apparently-unneeded ignore.

* Manually force out busted stubs package
@hauntsaninja
Copy link
Collaborator

I'm going to close this issue. I don't think type checkers should do the fully general thing of ignore type errors in try-except. Modern mypy is a lot happier here, it will allow imports of symbols if the type matches, or if the symbol has a previously declared type (and in particular, modules can now match Protocols). There is also now better support for type ignores in conditionally analysed code. For backports of stdlib features, using checks against sys.version_info also work very well.

See also #1153 (comment) / #8823 (comment)

@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Aug 27, 2023
@AlexWaygood
Copy link
Member

I agree that mypy definitely shouldn't support checked exceptions in general, but try/except ImportError is such a common idiom that I honestly think there's a pretty strong argument for special-casing it. It's true that you can switch to sys.version_info checks and mypy will be happy, and that's fine when you get used to it. But that's not an idiom that seems intuitive to anybody who's new to Python typing, in my experience, and the fact that type checkers don't understand the more common way of writing things seems very strange to them.

Even if the decision is that we shouldn't implement any special-casing here, in order to remain consistent about never understanding any exception handling, I still think we could add a better error message to point users towards what they should do instead.

This is one of the most popular feature requests on the issue tracker, and I think that's evidence that this is a real problem that users are facing.

@AlexWaygood AlexWaygood reopened this Aug 28, 2023
@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Aug 28, 2023

This issue was opened in 2016 and there's a lot that has changed since then. In particular, I fixed the most common case of this a couple months ago: where a conditional import redefines a symbol with exactly the same structural type. I think in #13969, but some other PRs were involved as well

Like I mention in the comment I link to, the issue is about providing guarantees about the rest of the code. type: ignore is cheap and if you break mypy's ability to reason about code elsewhere, you should have to add one. For instance, if you redefine a symbol with a different type or you have differing nominal types, mypy is right to complain.

I'm happy to leave issues open if we can tweak an error message, but I'm not seeing many additional things to implement that are a good combination of viable and sound, so a broad issue like this isn't super actionable. The main workable thing I can think of is doing something for typing + typing_extensions special forms.

@bdarnell
Copy link
Contributor Author

bdarnell commented Sep 9, 2023

This issue was opened in 2016 and there's a lot that has changed since then.

As the original issue author, I agree that a lot has changed. I no longer really need this, because the main case where it came up for me was in code that needed support for Python 2 and 3 simultaneously. I still have a few places with lingering type: ignore comments due to ImportError handling, but they're infrequent enough that I'm comfortable saying that the code should be changed instead of mypy.

@ghost
Copy link

ghost commented Sep 9, 2023

Python 2 may be no longer relevant, but typing_extensions and such is still relevant unless you only want to support the absolute latest Python version. It's not a dealbreaker but try/except does feel more Pythonic than checking sys.version_info.

@cebtenzzre
Copy link

typing_extensions and such is still relevant

In most cases, guarding your typing imports with if TYPE_CHECKING: and using from __future__ import annotations is sufficient. You can quote types in aliases and use TypeAlias. Inheriting from a type in typing may require some extra logic for runtime. But I haven't found myself using typing_extensions except for trying experimental features.

@TeamSpen210
Copy link
Contributor

That doesn't work for anything examining the types at runtime, and also for types used outside annotations (such as class definitions, decorators, typevarlike definitions, etc).

sambhav pushed a commit to argoproj-labs/hera that referenced this issue Aug 28, 2024
**Pull Request Checklist**
- [ ] Fixes #<!--issue number goes here-->
- [ ] Tests added
- [ ] Documentation/examples added
- [X] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
Currently, conditional imports that depend on Python version are using
`try/except ImportError` to import from `typing_extensions` on older
Python versions. This is not natively supported by typecheckers, forcing
use of error suppression, and losing potential validation of downstream
code. (Additionally, pyright is erroneously flagging a lot of code that
uses `Annotated` with "Call expression not allowed in type expression,
making it harder to spot real issues in VSCode.)

This PR refactors these conditional imports to use the recommended `if
sys.version_info >= (3, minor):` pattern instead.

See also: [mypy issue
comment](python/mypy#1393 (comment)),
[pyright issue
comment](microsoft/pyright#4160 (comment))

Signed-off-by: Alice Purcell <alicederyn@gmail.com>
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