Repository navigation
feat(typing): add type annotations to whoosh.filedb.structfile (#193) - #194
Conversation
Annotate StructFile, BufferFile, and ChecksumFile: - __init__ params typed (BinaryIO, Callable, str | None) - All write_* methods -> None - All read_* methods return their value type (int, float, bytes, array, Any) - All get_* positional helpers typed (int, int) -> (int | float | bytes | array) - get_string2 / get_string4 return tuple[bytes, int] - Pass-through wrappers (read, write, seek, tell, truncate, readline) use *args/ **kwargs: Any -> Any per issue spec - __exit__ uses type[BaseException] | None, BaseException | None, TracebackType | None (PYI036) - Callable moved to collections.abc (UP035) - No behaviour changes; ruff check/format clean; 23/23 tests pass Closes priya-sundaram-dev#193 Signed-off-by: webdevsamran <[email protected]>
priya-sundaram-dev
left a comment
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Thanks @webdevsamran — this is your third typing PR and it shows: clean, mechanical, behaviour-preserving annotations exactly matching the pattern from #172/#175/#179/#186. I re-ran everything locally against the checked-out branch:
ruff check+ruff format --check→ cleanpython -m mypy(the public-API smoke fixture) →Success: no issues foundpytest tests/test_misc.py tests/test_reading.py tests/test_writing.py→ 50 passed
A few things I especially liked: from __future__ import annotations so the StructFile/BufferFile forward refs resolve without quotes, Callable pulled from collections.abc (UP035), and the PYI036-correct __exit__ triple (type[BaseException] | None, BaseException | None, types.TracebackType | None). The *args: Any, **kwargs: Any -> Any pass-throughs are the right call for the thin wrappers.
Approving. One tiny nit (not blocking): line 255 in write_byte's docstring picked up a stray trailing backslash:
``file.write(chr(n))``.\Since it's an unescaped line-continuation in a plain docstring it silently merges the closing-quote line, so the rendered __doc__ ends with . + 8 spaces instead of a newline — a small behaviour change that slips past the "annotations-only" intent. Easiest fix is to just drop the \ so it reads ``file.write(chr(n))``. again.
Happy to squash that out myself when I merge so you don't have to round-trip — but if you'd rather push the one-char fix to keep your commit clean, go for it and I'll merge right after. Either way, thank you for another solid contribution to the typing effort (#121).
Signed-off-by: webdevsamran <[email protected]>
|
Good catch on the docstring line-continuation! Dropped the stray backslash in c53f10d so the commit history stays clean. Thanks for the quick review and approval! |
|
Nice — history's clean now. One last thing before I merge: CI's
>>> str(StructFile(io.BytesIO(), name=None))
TypeError: __str__ returned non-string (type NoneType)The minimal fix that keeps behavior for named files and satisfies def __str__(self) -> str:
return self._name if self._name is not None else ""That's a one-liner and fully in the spirit of this PR (making the types honest). Want to push it, or would you prefer I squash it in on merge with credit to you? Either's fine by me — just let me know. Thanks again for the careful work here. 🙏 |
Signed-off-by: webdevsamran <[email protected]>
|
Great catch by |
|
Merged — thank you, @webdevsamran! 🎉 This is exactly the kind of contribution I love to see: the annotations are clean, and your typing work surfaced (and fixed) a real latent bug in If you enjoyed this, |
2 participants
Summary
Adds type annotations to
whoosh.filedb.structfileas requested in #193. Same annotations-only pattern as the recently mergedutil.numeric(#172),util.text(#175),util.varints(#179), andutil.versions(#186) PRs.What changed
src/whoosh/filedb/structfile.py� annotations only, zero behaviour changes:StructFile.__init__:fileobj: BinaryIO,name: str | None,onclose: Callable[[StructFile], None] | Nonewrite_*methods: parameter types +-> Noneread_*methods:-> int,-> float,-> bytes, or-> arrayas appropriateget_string2/get_string4:-> tuple[bytes, int]read,write,seek,tell,truncate,readline):*args: Any, **kwargs: Any -> Anyper issue specBufferFile.__init__:buf: bytes | bytearray | memoryviewBufferFileandChecksumFilemethods fully annotatedCallablemoved tocollections.abc(ruff UP035)__exit__usestype[BaseException] | None,BaseException | None,types.TracebackType | None(ruff PYI036)CHANGELOG.md: entry added under[Unreleased] ? Typing.Verification
AI disclosure
Drafted with AI assistance (Gemini / Antigravity). All type signatures were derived directly from the method implementations and the suggested signatures in the issue body � no logic was altered.