Type annotations

[einarwar]

Hi there.

I came across this package when trying to zip large files in FastAPI on systems with restrictions on memory usage, and this package was just what i needed!

I noticed that there were no type-annotations, which made implementation slightly more difficult. To make use of this package easier, it could be beneficial to add type-annotations, at least for public functions. If this is something you think is good, I would be more than happy to provide a PR :)

[michalc]

Hi @einarwar,

Sounds great! If you're happy to raise a PR, that would be amazing.

In my head then in CI at https://github.com/uktrade/stream-zip/blob/main/.github/workflows/test.yml we would also run mypy on the code, and I guess the tests as well, and that should hopefully be enough to check the type annotations?

[michalc]

Next step: #122

[einarwar]

Ah… if we wanted to use Iterable[bytes] would that be a Python 3.8/Protocol thing? What would be the pre-3.8 alternatives, if any? Could we even have different types for different Python versions?

Iterable and the like should be supported since python 3.6 i believe. I was think of typing.Protocol https://peps.python.org/pep-0544/ . Since objects/classes are passed around quite a bit in the code, Protocols can provide type hints to which attributes or methods the object needs to implement to be valid.
For a concrete code example:

stream-zip/stream_zip.py

Lines 327 to 339 in d9d79b4

	def _zip_data(chunks, _get_compress_obj, max_uncompressed_size, max_compressed_size) -> Iterable[bytes]:
	uncompressed_size = 0
	compressed_size = 0
	crc_32 = zlib.crc32(b'')
	compress_obj = _get_compress_obj()
	for chunk in chunks:
	uncompressed_size += len(chunk)
	_raise_if_beyond(uncompressed_size, maximum=max_uncompressed_size, exception_class=UncompressedSizeOverflowError)
	crc_32 = zlib.crc32(chunk, crc_32)
	compressed_chunk = compress_obj.compress(chunk)
	compressed_size += len(compressed_chunk)

The _get_compress_obj could be typed as a Callable that returns a Protocol that implements the compress method.

(Happy for more PRs - suspect it might take a few to say, get it to the point of strict type checking?)

Having been through the process of typing an entire project that was previously untyped, my experience is that it is a long and iterative process. It has however saved me from several potential bugs, and i believe it is well worth it :)

[michalc]

The _get_compress_obj could be typed as a Callable that returns a Protocol that implements the compress method.

Ah I see! Although am I right that it would also have to have a "flush" method?

I have now made it the "nominal" (if that's the right word) type of 'zlib._Compress'. But to check really, is there much of a use case to use a Protocol here? Or is it sort of the principal of the thing - nothing in the code at runtime really requires a zlib._Compress - someone should be free to use another compressor if they want, so it's too strict to enforce it at the type level?

[michalc]

In any case, have now a, hopefully, type hinted stream-zip package (as of v0.0.80 really). It should cause mypy to up with errors like the following if you pass stream_zip the wrong thing for example:

Argument 1 to "stream_zip" has incompatible type "int"; expected "Iterable[tuple[str, datetime, int, Method, Iterable[bytes]]]

[einarwar]

Wow, I am really impressed at what you did during the weekend. I was hoping to be able to contribute, but you were too fast 😅. This is really nice, thank you so much!

I used the package for something similar to the example mentioned in #31 , and with a fully typed external API (and internal for that matter) things work really smooth :)