-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add type stub for the lzma module #1844
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
Conversation
This doesn't record the true hierarchy of the type, though avoids errors from mypy that 'Argument 1 of "read" incompatible with supertype "BufferedIOBase"'.
stdlib/3.3/lzma.pyi
Outdated
errors: Optional[str] = ..., | ||
newline: Optional[str] = ...) -> IO[Any]: ... | ||
def compress(data: bytes, format: int = ..., check: int = ..., preset: Optional[int] = ..., filters: Optional[_FilterChain] = ...) -> bytes: ... | ||
def decompress(data: bytes, format: int = ..., memlimit: Optional[int]= ..., filters: Optional[_FilterChain] = ...) -> bytes: ... |
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.
missing a space before one of the =
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.
Good spot, thanks! 0339a8f
stdlib/3.3/lzma.pyi
Outdated
else: | ||
_PathOrFile = str | ||
|
||
_FilterChain = List[Dict[str, Any]] |
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.
Does this really have to be a List of Dicts? Is an Iterable of Mappings also acceptable?
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.
Good point; from the source it looks like anything dict-like should be fine; a824afe.
While we're thinking about this: the dictionary must always have at least an 'id'
key defined -- is there a way to add that to the type signature?
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.
Possibly with TypedDict, but it's fine to not worry about it for now.
|
stdlib/3.3/lzma.pyi
Outdated
_PathOrFile = Union[str, bytes, IO[Any], PathLike[Any]] | ||
elif sys.version_info >= (3, 3): | ||
_PathOrFile = Union[str, bytes, IO[Any]] | ||
else: |
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.
Why do you need this case? This module doesn't exist before 3.3.
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.
2ae4a6d; I copy/pasted from the bz2 stubs but failed to remove this.
stdlib/3.3/lzma.pyi
Outdated
else: | ||
_PathOrFile = str | ||
|
||
_FilterChain = List[Mapping[str, Any]] |
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.
From https://docs.python.org/3/library/lzma.html#specifying-custom-filter-chains, it sounds like this can actually be any Sequence.
This is important because List is invariant, so if you specify the type like this, users are only allowed to pass lists of exactly Mapping[str, Any]
, not lists of objects of some subclass of Mapping[str, Any]
.
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.
Thanks! checking the C source these can indeed be Sequences: fc4eb12
MODE_NORMAL: int | ||
PRESET_DEFAULT: int | ||
PRESET_EXTREME: int | ||
|
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.
You're missing is_check_supported
from https://docs.python.org/3/library/lzma.html#miscellaneous, and the exception class LZMAError
.
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.
stdlib/3.3/lzma.pyi
Outdated
def tell(self) -> int: ... | ||
|
||
|
||
def open(filename: _PathOrFile = ..., |
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.
The first argument doesn't have a default.
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.
D'oh ff7fe24
lzma is only in 3.3 and later, so the elif was actually already an else.
Thanks for the contribution! |
* python/master: (269 commits) allow Optional[float] for asyncio.wait() timeout arg (python#1860) Clean up the pytype blacklist. (python#1851) filter function: make the callable of the first overload non-optional so that mypy is able to select the second overload for the case. (python#1855) Accept Text in distutils *Version classes (python#1854) Add attrs library (python#1838) Add type stub for the lzma module (python#1844) Add ImportError attributes name, path for Python 3.3+ (python#1849) Add 'message' to click.decorators.version_option (python#1845) PEP 553 and PEP 564 (python#1846) Adding py36 additions to datetime module (python#1848) Remove incomplete thrift stubs that cause false positives. (python#1827) adding curses.ascii, curses.panel and curses.textpad modules (python#1792) Fix requests session hooks type (python#1794) Add StringIO.name and BytesIO.name (python#1802) Fix werkzeug environ type (python#1831) Change the return type of unittest.TestCase.fail() to NoReturn (python#1843) _curses.tparm works on bytes, not str (python#1828) Refine types for distutils.version (python#1835) Add stub for stat.filemode (python#1837) modify pytype_test to run from within pytype too, and support python3 (python#1817) ...
Note to reviewer: there are a bunch of places where the
lzma
module claims to be happy with a "bytes-like object" which I wans't sure how to represent and have used plainbytes
instead.If there is an easy way to do that I'll also open a PR for the
bz2
stub, which similarly usesbytes
where "bytes-like" is more correct.