Repository navigation
Add type annotations - #264
Conversation
root.iter() also yields comments and processing instructions, whose tag is a factory function rather than a string, so calling child.tag.rfind() raised AttributeError. Skip non-string tags. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The test was missing its test_ prefix, so pytest never collected it, and it shadowed the binary_regex_match helper star-imported from talon.signature.learning.helpers. Its prog parameter was unused. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Annotate the talon package, train.py, and tests, and check them with mypy --strict in a new CI job. Ship a py.typed marker so downstream type checkers use the inline annotations (PEP 561). `from __future__ import annotations` lets modern annotation syntax work on Python 3.7. XPath extension function contexts are described by a TYPE_CHECKING-only Protocol, since typing.Protocol needs Python 3.8. Small code changes needed for type checking: - Give variables that changed type partway through separate names. - Return False instead of falling through to None from cut_* functions, and return None explicitly from is_splitter. - Return a real bool from is_signature_line instead of a one-element numpy array. - Build the parser with lxml.html.html5parser.HTMLParser, the same html5lib parser and lxml tree builder that the stubs expect. - Check for a missing parent before removing elements in cut_by_id and html_tree_to_text. - Re-export talon.signature.extract explicitly, so it works under --no-implicit-reexport. - Fix the existing annotations of cssselect and _contains_charset_spec. Test changes: pass True rather than 1 for sender_known, patch the regex module directly rather than through talon's namespace, and simplify body_iterator, whose str branch could never run. mypy, types-lxml, types-regex, and types-six are added to test-requirements.txt for Python 3.10 and newer. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| class _XPathContext(Protocol): | ||
| """The context lxml passes to XPath extension functions.""" | ||
|
|
||
| @property | ||
| def context_node(self) -> _Element: ... |
There was a problem hiding this comment.
I opened an upstream PR for this (but writing this as a Protocol means we don’t need to block on it):
obukhov-sergey
left a comment
obukhov-sergey
left a comment
There was a problem hiding this comment.
talon/py.typed opts every downstream typechecker into these annotations, but the stub packages live only in test-requirements.txt (types-lxml, types-regex, types-six). A core install does not pull them in. Public signatures use lxml.etree._Element, and utils.py imports regex and six, so mypy on a dependent project fails until those stubs are installed. Talon's own [tool.mypy.overrides] for sklearn and joblib apply only when Talon is the project being checked.
The same import also drags in the ML extra. talon/signature/__init__.py re-exports extract like this:
if TYPE_CHECKING:
from talon.signature import extraction
from talon.signature.extraction import extract as extractType checkers follow that import. extraction.py imports numpy at module level, so import talon.signature.bruteforce — which is supported without talon[ml] — fails typechecking unless numpy is installed.
Stub extract in the TYPE_CHECKING block instead of importing extraction, and document a typing extra for types-lxml, types-regex, and types-six. Talon's CI already typechecks extraction.py directly, so the signatures stay checked without forcing that import on core users.
|
I can’t reproduce the $ echo 'import talon.signature.bruteforce' >test.py
$ echo 'reveal_type(talon.signature.bruteforce.extract_signature)' >>test.py
$ uv run --with=mypy,talon@git+https://github.com/andersk/talon.git@type-annotations mypy --strict test.py
test.py:2: note: Revealed type is "def (msg_body: str) -> tuple[str, str | None]"
Success: no issues found in 1 source file
$ uv run --with=pyright,talon@git+https://github.com/andersk/talon.git@type-annotations pyright test.py
/tmp/test.py
/tmp/test.py:2:13 - information: Type of "talon.signature.bruteforce.extract_signature" is "(msg_body: str) -> tuple[str, str | None]"
0 errors, 0 warnings, 1 informationType checkers resolve imported types to
So I think it’s unnecessary to add any public dependencies here. |
|
Sure, if you like. It doesn’t affect users either way: |
Annotate the
talonpackage,train.py, andtests, and check them withmypy --strictin a new CI job. Ship apy.typedmarker so downstream type checkers use the inline annotations (PEP 561).