feat: add type hinting to s3.open() and main open()
#885
Conversation
There was a problem hiding this comment.
CI should really get upgraded and have ruff for checking and pyright for type validations
I would love an astral stack upgrade! They also built a type checker that I expect will supercede mypy/pyright over time: https://github.com/astral-sh/ty
Overall it would be great to have some CI to cover this PR before merging.
Big bonus would be to differentiate r/w/a return types (see docstring I mentioned in the issue, although I don't know a project that has implemented this properly).
| encoding: None = ..., | ||
| errors: None = ..., | ||
| newline: None = ..., | ||
| closefd: bool = ..., |
There was a problem hiding this comment.
can't all these be omitted? ie the overloads only contain uri and mode? also the type hints here (None) are differing from below
| compression: str = ..., | ||
| transport_params: dict[str, Any] | None = ..., | ||
| ) -> TextIO: ... | ||
| @overload |
| errors: str | None = None, | ||
| newline: str | None = None, | ||
| closefd: bool = True, | ||
| opener: object | None = None, |
There was a problem hiding this comment.
closefd and opener are completely ignored, not even propagated to _shortcut_open
| client: object | None = None, | ||
| client_kwargs: dict | None = None, | ||
| writebuffer: IO[bytes] | None = None, | ||
| ) -> BinaryIO: |
There was a problem hiding this comment.
why IO[bytes] and not BinaryIO for writebuffer?
|
first step towards |
2 participants
Motivation
Related to #518. Not a complete fix, but makes some progress.
Tests
CI should really get upgraded and have
rufffor checking andpyrightfor type validations.