Repository navigation
Import of typing should be conditional #178
Description
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
Activity
Make typing optional
bbc2 commentedon May 16, 2019
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.
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 commentedon May 16, 2019
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
BindingNamed 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 toextras_requireso 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 commentedon May 16, 2019
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)
Make typing optional
bbc2 commentedon Jun 10, 2019
I've been thinking about this and I wondered why you would want this library to be more complex instead of vendoring
typing.pyand accepting the extra import. I still don't buy the performance argument because I'm almost sure that importingtypingis 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).To avoid likely misconceptions:
techalchemy commentedon Jun 12, 2019
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.
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
typinginstalled.Everyone who installs
python-dotenvdirectly and/or who hastypinginstalled will still benefit from type hints, which will continue to work even with the conditional. The complexity should mean only anifstatement around your typing imports and that should be itMake typing optional (#179)
Make typing optional (theskumar#179)