Skip to content

feat: added stubs for type checkers #223#295

Open
Anshuman-37 wants to merge 4 commits into
python-lz4:masterfrom
Anshuman-37:master
Open

feat: added stubs for type checkers #223#295
Anshuman-37 wants to merge 4 commits into
python-lz4:masterfrom
Anshuman-37:master

Conversation

@Anshuman-37

@Anshuman-37 Anshuman-37 commented Jan 17, 2025

Copy link
Copy Markdown

Hi, just added types. Please point out mistakes if any. Closes #223

@jonathanunderwood

Copy link
Copy Markdown
Member

Thanks for this. At present this only annotates the function/method return types, and doesn't cover the block bindings, so isn't really complete.

@Anshuman-37

Copy link
Copy Markdown
Author

Do you mean about the types for the parameters in the functions for example
def get_block(self, stream: Union[str, bytes, memoryview]) -> Union[bytes, bytearray]:

@jmg-duarte

jmg-duarte commented Jan 23, 2025

Copy link
Copy Markdown
Contributor

@jmg-duarte jmg-duarte left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! As Jonathan said, only the block left to go!

Comment thread lz4/frame/__init__.py Outdated

if self._started is False:
self._context = create_compression_context()
self._context: Any = create_compression_context()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be a "regular" Any, in Python this represents literally ANY object. This means that a str could be "compatible" with this.

A safer approach would be to use NewType

CompressionContext = NewType("CompressionContext", Any)

Comment thread lz4/frame/__init__.py Outdated
_MODE_READ: int = 1
# Value 2 no longer used
_MODE_WRITE = 3
_MODE_WRITE: int = 3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same applies to the others

Suggested change
_MODE_WRITE: int = 3
_MODE_WRITE: Literal[3] = 3

Comment thread lz4/frame/__init__.py Outdated
self._fp.flush()

def seek(self, offset, whence=io.SEEK_SET):
def seek(self, offset: int, whence: int = io.SEEK_SET) -> int:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If whence is only SEEK values, then the right type is: Union[Literal[0], Literal[1], Literal[2]] as per https://github.com/python/cpython/blob/d674792ba773d78d4ed71698b7d47fafb8842d89/Lib/io.py#L65-L67

Comment thread lz4/frame/__init__.py Outdated
auto_flush=False,
return_bytearray=False,
source_size=0):
def open(filename, mode: str = "rb",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to others, the right type for open would be: Union[Literal["w"], ...] based on https://github.com/python-lz4/python-lz4/pull/295/files#diff-c82af95b2ce244d45d0ba30e51ab0da71df329e418ae1cb3870e024ce4ebb99eR818-R820

When adding this, don't forget to make your life easier by doing:

OpenModes = Union[Literal["w"], ...]

Comment thread lz4/frame/__init__.py
return result

def flush(self):
def flush(self) -> Union[bytes, bytearray]:

This comment was marked as resolved.

This comment was marked as resolved.

@jmg-duarte

Copy link
Copy Markdown
Contributor

Ah, don't forget to add mypy to the CI!

@jmg-duarte jmg-duarte left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just a small improvement

Comment thread lz4/stream/__init__.py Outdated
def __init__(self, strategy, buffer_size, return_bytearray=False, store_comp_size=4, dictionary=""):

def __init__(self, strategy: str, buffer_size: int, return_bytearray: bool = False, store_comp_size: int = 4,
dictionary: Union[str, bytes, memoryview] = ""):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dictionary: Union[str, bytes, memoryview] = ""):
dictionary: BytesLike = ""):

And up in the file:

BytesLike = Union[str, bytes, memoryview]

Apply to the others too, this makes it easier to not f-up when writing the same union types

@Anshuman-37

Copy link
Copy Markdown
Author

Okay I have updated the code please have a look @jmg-duarte and @jonathanunderwood

@jonathanunderwood

Copy link
Copy Markdown
Member

Conflicts to resolve.

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.

Add stubs for type checkers (mypy, pytype, etc)

3 participants