Skip to content

fix(typing): preserve decorated function type signatures with ParamSpec#376

Open
gencurrent wants to merge 6 commits intopython-cachier:masterfrom
gencurrent:fix/preserve-decorator-type-signature
Open

fix(typing): preserve decorated function type signatures with ParamSpec#376
gencurrent wants to merge 6 commits intopython-cachier:masterfrom
gencurrent:fix/preserve-decorator-type-signature

Conversation

@gencurrent
Copy link
Copy Markdown
Contributor

@gencurrent gencurrent commented Apr 13, 2026

I encounter this issue with Pyrefly constantly: the declared return type of the decorated function / method does not match the expected return type, so I have to add # ... ignore ... comments.

The @cachier decorator erases all type information from decorated functions. Because cachier() has no return type annotation and no generic type variables, type checkers see every decorated function -- both its signature and return value -- as Any:

@cachier()
def my_func(x: int) -> str:
    return str(x)

reveal_type(my_func)      # Any
reveal_type(my_func(5))   # Any

This breaks IDE autocomplete, disables type error detection on wrong arguments, and forces downstream typed codebases to use stubs or type: ignore.

Changes

  • src/cachier/core.py: Add ParamSpec/TypeVar generics to cachier(), _cachier_decorator(), and func_wrapper so the original function signature is preserved through the decorator
  • tests/test_typing.py: 12 mypy-based tests covering sync, async, complex signatures, wrong-arg rejection, and decorator argument variants
  • tests/requirements.txt: Add mypy as a test dependency
    After:
reveal_type(my_func)      # (x: int) -> str
reveal_type(my_func(5))   # str

⚠️ The docstring step is skipped in the CI pipeline.

@gencurrent gencurrent requested a review from shaypal5 as a code owner April 13, 2026 22:19
Copy link
Copy Markdown
Member

@shaypal5 shaypal5 left a comment

Choose a reason for hiding this comment

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

Looks good. Let's wait and see what CoPilot says. :)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to improve Cachier’s static typing experience by preserving decorated function signatures/return types (especially for tools like mypy/Pyrefly) using ParamSpec + TypeVar, and adds a mypy-driven test suite to validate the behavior.

Changes:

  • Add ParamSpec/TypeVar generics to cachier() and its internal decorator/wrapper to preserve function signatures through decoration.
  • Add tests/test_typing.py which runs mypy programmatically to assert signature/return-type preservation for sync/async functions and some decorator argument variants.
  • Add mypy to tests/requirements.txt so the typing tests can run in CI.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
src/cachier/core.py Introduces generic typing for the decorator/wrapper to preserve callable signatures and return types.
tests/test_typing.py Adds mypy-based tests that validate the preserved typing behavior via reveal_type and error assertions.
tests/requirements.txt Adds mypy as a test dependency to enable running the typing tests.

Comment thread tests/test_typing.py
Comment thread tests/test_typing.py Outdated
Comment thread tests/test_typing.py
Comment thread tests/test_typing.py
Comment thread src/cachier/core.py Outdated
Comment thread src/cachier/core.py
@gencurrent
Copy link
Copy Markdown
Contributor Author

Now there's a test fail with
failed to connect to the docker API at npipe:////./pipe/docker_engine; check if the path is correct and if the daemon is running: open //./pipe/docker_engine: The system cannot find the file specified.

@shaypal5
Copy link
Copy Markdown
Member

Now there's a test fail with failed to connect to the docker API at npipe:////./pipe/docker_engine; check if the path is correct and if the daemon is running: open //./pipe/docker_engine: The system cannot find the file specified.

I don't know. I'm looking at it now, and the only red flow is codecov's. Please take a look at CoPilot's comments, treat anything valid, resolve as rejected anything that's not, raise patch test coverage to 100% (it's currently at 96.42%), and i'll merge this sucker.

@gencurrent
Copy link
Copy Markdown
Contributor Author

Now there's a test fail with failed to connect to the docker API at npipe:////./pipe/docker_engine; check if the path is correct and if the daemon is running: open //./pipe/docker_engine: The system cannot find the file specified.

I don't know. I'm looking at it now, and the only red flow is codecov's. Please take a look at CoPilot's comments, treat anything valid, resolve as rejected anything that's not, raise patch test coverage to 100% (it's currently at 96.42%), and i'll merge this sucker.

Perhaps it failed just once yesterday between some of my commits.
Thank you : ) I replied to all the comments and fixed the relevant ones and replied to each.

@gencurrent
Copy link
Copy Markdown
Contributor Author

gencurrent commented Apr 17, 2026

The pre-commit.ci - pr failure is unrelated to this PR - it's a build error in untokenize (a transitive dep of docformatter v1.7.7 pinned in .pre-commit-config.yaml) on Python 3.12+, where ast.Constant.s was removed.

Note that the GitHub Actions pre-commit job passes (different Python version). Could you either:

  1. Bump docformatter in .pre-commit-config.yaml to a newer rev (outside the scope of this PR), or
  2. Merge anyway since the real pre-commit check passes?

@gencurrent
Copy link
Copy Markdown
Contributor Author

I think it's docformatter: cachier uses it, but the project (https://github.com/PyCQA/docformatter) hasn't been released for almost a year, since May of 2025.

docformatter v1.7.7 transitively pulls untokenize, whose setup.py uses
ast.Constant.s (removed in Python 3.12+) and fails to install on
pre-commit.ci's runners. Skip the hook on pre-commit.ci until
docformatter v1.7.8 ships (which drops the untokenize dep). The
pre-commit GitHub Actions job still runs docformatter, so coverage of
the hook isn't lost.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gencurrent
Copy link
Copy Markdown
Contributor Author

@shaypal5 Ready!
But with one caveat: ⚠️ The `docstring` step is skipped in the CI pipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants