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

Remove stream kwarg of read_env() #114

Merged
merged 1 commit into from
Dec 3, 2019
Merged

Remove stream kwarg of read_env() #114

merged 1 commit into from
Dec 3, 2019

Conversation

hukkin
Copy link

@hukkin hukkin commented Dec 2, 2019

Remove the stream kwarg of read_env().

Firstly, the kwarg is incorrectly type annotated. It is annotated as str and is passed as a kwarg to dotenv.load_dotenv(). That kwarg of dotenv.load_dotenv(), however, takes an io.StringIO, not a string.

Secondly, I believe the stream kwarg never does anything here. dotenv.load_dotenv() only uses the stream kwarg if its first kwarg (dotenv_path) is falsy. It seems to me that we always pass a truthy value to that, so stream is never used. Therefore I suggest we remove it.

I recommend running mypy with access to site-packages and their typehints in CI (mypy in pre-commit runs in a venv with no site-packages, unless dependencies listed in .pre-commit-config.yaml). It would catch this and a few other issues.

@sloria
Copy link
Owner

sloria commented Dec 3, 2019

Good catch!

As for running mypy on dependencies, feel free to send a PR for that; I won't have time to look into it in the short term.

@sloria sloria merged commit 66d0636 into sloria:master Dec 3, 2019
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 this pull request may close these issues.

3 participants