Import of typing should be conditional

[techalchemy]

Hey -- so we vendor this library over in pipenv and I recently updated to the latest release before realizing that there is now a hard dependency on typing. We don't vendor typing and I'd rather avoid that, but it's also not strictly necessary to put unguarded typing imports everywhere. They are slow, for one thing, or they can be, and they break on systems where typing isn't available.

What I often do is include a utility method that sets a variable if the type checker is running, something like this (in utils.py or environment.py):

def is_type_checking():
    try:
        from typing import TYPE_CHECKING
    except ImportError:
        return False
    return TYPE_CHECKING


IS_TYPE_CHECKING = os.environ.get("MYPY_RUNNING", is_type_checking())

If that solution makes sense here, I'm happy to PR it. I'll open a PR and feel free to merge or close as needed

[bbc2]

We have indeed introduced a dependency on typing (as a separate package for Python < 3.5). This provides authors and users of Python-dotenv with static type checking.

it's also not strictly necessary to put unguarded typing imports everywhere. They are slow, for one thing, or they can be, and they break on systems where typing isn't available.

Not necessary but idiomatic as far as I'm aware. However, I didn't know these imports could be slow or break. Do you have more information on that? I quite like static type checking so I would be sad if we had to go through more hoops to benefit from it (supporting Python 2 is already quite complicated).

[techalchemy]

Sorry, actual evaluations are slow. Imports are just as slow as imports are — not incredibly bad but not inconsequential.

Type hints are useful in the specific case of the Binding Named Tuple, but this is really only helpful for developers working on the library. The type checker doesn’t actually help during runtime, so having an install requirement on something that isn’t technically needed to run the library is something that is typically best relegated to extras_require so you and any users can make a choice as to whether it will be installed.

The implementation is not all that common but we use it in a number of packaging authority libraries and I’ve been encouraging the practice more. It’s not too bad, I have a PR open at #179 with the changes. Happy to adjust if you have concerns

[techalchemy]

Also I should mention static type checking is great and the changes should not break type checking for anyone on python 3.5 or above or with the typing package installed (currently anyone who installs this library)

[bbc2]

I've been thinking about this and I wondered why you would want this library to be more complex instead of vendoring typing.py and accepting the extra import. I still don't buy the performance argument because I'm almost sure that importing typing is almost nothing in comparison to all the things that Pipenv does. What I didn't know, however, is that vendoring typing is harder than it seems (pypa/pip#4545 (comment)). With that in mind, I'll have a look at your MR (I thought that the need for vendoring was unique to Pip but I will assume Pipenv has a valid reason for also doing it).

Type hints are useful in the specific case of the Binding Named Tuple, but this is really only helpful for developers working on the library.

To avoid likely misconceptions:

• The named tuple isn't the only useful type hint.
• This library isn't the only one benefitting from those type hints: they are also useful for libraries that import Python-dotenv, see https://www.python.org/dev/peps/pep-0561/.

[techalchemy]

I've been thinking about this and I wondered why you would want this library to be more complex instead of vendoring typing.py and accepting the extra import. I still don't buy the performance argument because I'm almost sure that importing typing is almost nothing in comparison to all the things that Pipenv does

The argument is more to persuade you that it is not a bad idea, and that there are valid reasons for putting an if statement around your typing imports. This does not help pipenv with performance, this helps pipenv from becoming broken where typing is not imported. It is unfortunately not actually possible to depend on vendored typing imports for this for a number of reasons or we would do that.

The named tuple isn't the only useful type hint.

I'm not suggesting that it's the only useful type hit, it was just the only type hint that I had to put a fallback implementation around as a result of the changes I made which will be used in case the user doesn't have typing installed.

This library isn't the only one benefitting from those type hints: they are also useful for libraries that import Python-dotenv, see python.org/dev/peps/pep-0561.

Everyone who installs python-dotenv directly and/or who has typing installed will still benefit from type hints, which will continue to work even with the conditional. The complexity should mean only an if statement around your typing imports and that should be it