Skip to content

Conversation

@adamtheturtle
Copy link

This PR adds type hints to the Python snowballstemmer package and includes a py.typed marker file for PEP 561 compliance.

Changes

  • Add type hints to among.py, basestemmer.py, stemwords.py, testapp.py
  • Update create_init.py to generate typed __init__.py with annotations for algorithms() and stemmer() functions
  • Add py.typed marker file for PEP 561 compliance
  • Update setup.py with package_data to include py.typed in installed package
  • Update MANIFEST.in and GNUmakefile to include py.typed in distribution

Benefits

This allows users of the snowballstemmer package to benefit from static type checking with mypy, pyright, and other type checkers. IDEs will also provide better autocompletion and documentation.

Compatibility

  • Uses from __future__ import annotations for forward reference support
  • Uses TYPE_CHECKING guards to avoid runtime import overhead
  • Compatible with Python 3.3+ (the existing minimum supported version)

- Add type hints to among.py, basestemmer.py, stemwords.py, testapp.py
- Update create_init.py to generate typed __init__.py
- Add py.typed marker file for PEP 561 compliance
- Update setup.py, MANIFEST.in, and GNUmakefile to include py.typed
Copy link
Contributor

@mitya57 mitya57 left a comment

Choose a reason for hiding this comment

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

Uses from __future__ import annotations for forward reference support

According to the note in Python documentation, maybe it's a good idea to not use it in new code, and use quoted literals (and remove them once the minimum Python version is bumped to 3.14, which has deferred evaluation of annotations).

However, I don't see this import actually used in your PR anyway.

Uses TYPE_CHECKING guards to avoid runtime import overhead

And I don't see any use of TYPE_CHECKING, either.

The input file consists of a list of words to be stemmed, one per
line. Words should be in lower case, but (for English) A-Z letters
are mapped to their a-z equivalents anyway. If omitted, stdin is
are mapped to their a-z aquivalents anyway. If omitted, stdin is
Copy link
Contributor

Choose a reason for hiding this comment

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

This change looks wrong.

@adamtheturtle adamtheturtle closed this by deleting the head repository Dec 29, 2025
@ojwb
Copy link
Member

ojwb commented Jan 4, 2026

@adamtheturtle closed this by deleting the head repository last week

I'm going to assume this means you don't want to continue with this. If you actually do, please restore your fork and address the points raised.

This allows users of the snowballstemmer package to benefit from static type checking with mypy, pyright, and other type checkers. IDEs will also provide better autocompletion and documentation.

That's rather shallow and really doesn't explain some of the changes - for example, you add type hints to stemwords.py which is an example/test program rather than part of the API the package provides.

(I suspect the summary is LLM generated, and that really isn't very useful here - we're going to fully read any patch we merge anyway, so they really key part is to explain exactly what you're actually trying to achieve with a proposed change. At least with the current state of LLMs that is much better done directly by the person who made the changes. If it was human written, please focus on tell us why rather than what.)

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.

3 participants