Skip to content

Add missing return type hints to BaseTokenizer.save, save_model, to_str#2211

Open
RudrenduPaul wants to merge 1 commit into
huggingface:mainfrom
RudrenduPaul:fix/base-tokenizer-return-type-hints
Open

Add missing return type hints to BaseTokenizer.save, save_model, to_str#2211
RudrenduPaul wants to merge 1 commit into
huggingface:mainfrom
RudrenduPaul:fix/base-tokenizer-return-type-hints

Conversation

@RudrenduPaul

Copy link
Copy Markdown

What this PR does

Adds missing return type hints to three methods in
bindings/python/py_src/tokenizers/implementations/base_tokenizer.py:

  • save_model(self, directory: str, prefix: Optional[str] = None) -> List[str]
  • save(self, path: str, pretty: bool = True) -> None
  • to_str(self, pretty: bool = False) -> str

This is a self-identified typing-consistency fix (no linked issue). Every
other method in this class already carries a return type hint; these three
were the only ones missing one. to_str's own docstring already documents
"Returns: str", so the annotation was simply never added to the signature.

Why these types are correct

Each method is a thin wrapper delegating to the compiled Rust bindings, and
the return type was verified against the auto-generated .pyi stubs
(bindings/python/py_src/tokenizers/__init__.pyi and
bindings/python/py_src/tokenizers/models.pyi), which are the source of
truth for the underlying Rust-exposed signatures:

  • save_model returns self._tokenizer.model.save(directory, prefix=prefix).
    Model.save(self, /, folder: str, prefix: str | None = None, name: str | None = None) -> "list[str]"
    per models.pyi — hence List[str] (the file already imports List from
    typing).
  • save returns self._tokenizer.save(path, pretty).
    Tokenizer.save(self, /, path: str, pretty: bool = True) -> "None" per
    __init__.pyi — hence None.
  • to_str returns self._tokenizer.to_str(pretty).
    Tokenizer.to_str(self, /, pretty: bool = False) -> "str" per
    __init__.pyi — hence str, matching the existing docstring.

Scope note: not a duplicate of #1928 / issue #1927

#1928 (merged, closing issue #1927) added type hints
and an automatic stub generator for the compiled PyO3 bindings (.pyi
files, stub.py, stub-gen tool). It did not touch the pure-Python
implementations/ package, so base_tokenizer.py's three unannotated
methods were left untouched by that effort. Open PR #1932 ("fix: added type
hints in .py files") likewise only touches benches/examples/scripts/tests,
not base_tokenizer.py. Confirmed via gh pr view file lists before
starting this change.

No behaviour change

Type-hint-only change. No logic was modified.

Testing

  • ruff check bindings/python/py_src/tokenizers/implementations/base_tokenizer.py — all checks passed.
  • ruff format --check bindings/python/py_src/tokenizers/implementations/base_tokenizer.py — passes, file already formatted.
  • python3 -c "import ast; ast.parse(...)" on the modified file — passes.

Note: Claude Code was used to assist in drafting this change. All
changes were reviewed by the submitter.

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.

1 participant