-
Notifications
You must be signed in to change notification settings - Fork 432
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
Fix type hint for dotenv_path var, add StrPath alias #432
Conversation
src/dotenv/main.py
Outdated
@@ -305,7 +305,7 @@ def _is_interactive(): | |||
|
|||
|
|||
def load_dotenv( | |||
dotenv_path: Union[str, os.PathLike, None] = None, | |||
dotenv_path: Union[str, 'os.PathLike[str]', 'os.PathLike[bytes]', None] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this better: Union[str, os.PathLike[Union[str, bytes]], None]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather use os.PathLike[str], os.PathLike[bytes]
, as it is done in typeshed
. We could even define a TypeAlias
here and use Optional
instead of Union[..., None]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotenv_path: str | os.PathLike[str | bytes] | None = None
is compact and nice, but currently supported from Python 3.10 and above. So yes typing.Optional would also be fine.
from typing import Optional, Union
...
dotenv_path: Optional[Union[str, os.PathLike[Union[str, bytes]]] = None
Anything needed to make this pr happen? :) Came here for this exact problem :) |
Based on PR feedback and typeshed's type hint for the built-in open() function: https://github.com/python/typeshed/blob/e2d67bf7034f68c07bd35150247e58e0817725d9/stdlib/builtins.pyi#L1421
These paths can flow into `shutil.move`, which does not accept byte paths or (int) file descriptors. See python/typeshed#6832
@eaftan I think a type alias could be used, as the |
And use it consistently in main.py.
Done. The CI seems to be failing with a coverage error, but I think that's not my fault -- it seems to have failed on the most recent commit to main as well: https://github.com/github/entitlements/pull/56799 |
Fixes #431