type hint FunctionWrapper

[grayjk]

Create a test case (testcase.py):

import wrapt

def x(source: str, dest: str) -> int:
    return 1

def wrapper(*args, **kwargs):
    pass

x = wrapt.FunctionWrapper(x, wrapper)
reveal_type(x)

x(4, None)

With a pyproject.toml:

[tool.mypy]
disallow_untyped_calls = true
follow_untyped_imports = true

Run mypy on the test case with wrapt from the develop branch:

$ mypy testcase.py 
testcase.py:9: error: Call to untyped function "FunctionWrapper" in typed context  [no-untyped-call]
testcase.py:10: note: Revealed type is "wrapt.wrappers.FunctionWrapper"
testcase.py:12: error: Argument 1 to "__call__" has incompatible type "int"; expected "_FunctionWrapperBase"  [arg-type]
testcase.py:12: error: Argument 2 to "__call__" has incompatible type "None"; expected "_FunctionWrapperBase"  [arg-type]
Found 1 error in 1 file (checked 1 source file)

Now run mypy with wrapt from the type-hint-func-wrapper branch:

$ mypy testcase.py 
testcase.py:10: note: Revealed type is "def (source: builtins.str, dest: builtins.str) -> builtins.int"
testcase.py:12: error: Argument 1 has incompatible type "int"; expected "str"  [arg-type]
testcase.py:12: error: Argument 2 has incompatible type "None"; expected "str"  [arg-type]
Found 2 errors in 1 file (checked 1 source file)

Relates to #258

[grayjk]

I thought adding type hints incrementally might be more amenable

This diff follows the guidance from https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators

The printing_decorator example using legacy syntax (Python 3.11 and earlier)

[GrahamDumpleton]

Part of the reason for trying to catch up with backlog of stuff in wrapt such as removing any Python 2.X code and increment to new major version has been to prepare for finally adding the type hint stuff.

Because I don't know enough about subtleties of type hints I still have couple of points of concern before charging into it.

The first is whether now that Python 2.X and older Python 3.X version support has been removed it is better to include the type hints directly in the Python code. I am still not sure whether this is possible though since most of the time people would be relying on the C extension. Can tools defer to the hints in the Python code even though it may be implemented in the C code.

The second is how to test things. I have been uncomfortable with adding it all without a way to confirm that it is done properly and that I don't break it over time. The comment above with the mypy example is the first I have seen that may be able to add some sort of tests to confirm things are okay.

Can you see a strategy for how we could add mypy driven tests as part of repository that can trigger locally and from GitHub actions?

Is using separate .pyi files still the best approach?

Also, to keep this going, it may be better that I create a feature branch in GitHub and we create PRs and merge against that. That way the whole branch can be checked out more easily to work on it incrementally and test it and know not upsetting develop branch.

[grayjk]

I used a .pyi file based on https://peps.python.org/pep-0484/#stub-files. Since wrapt includes a C extension, it would fall into the first use case for a stub file.

A way forward for testing might be https://github.com/TypedDjango/pytest-mypy-plugins. The attrs project uses it: https://github.com/python-attrs/attrs/blob/main/tests/test_mypy.yml. If this approach looks good to you, I'll take a stab at adding it to this PR.

[grayjk]

in commit 0126ea1, I added a test that uses pytest-mypy-plugins. Let me know if you like that format

[grayjk]

see https://typing.python.org/en/latest/reference/quality.html for some other approaches to testing

[GrahamDumpleton]

Looks like I got a bunch of stuff mypy picks up on before we even start with changes that need to sort out. 🤣

src/wrapt/wrappers.py:77: error: Unsupported dynamic base class "with_metaclass"  [misc]
src/wrapt/wrappers.py:346: error: Signatures of "__ipow__" and "__pow__" are incompatible  [misc]
src/wrapt/arguments.py:11: error: Module "inspect" has no attribute "formatargspec"  [attr-defined]
src/wrapt/__wrapt__.py:24: error: Skipping analyzing "wrapt._wrappers": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/wrapt/__wrapt__.py:24: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
src/wrapt/__wrapt__.py:24: error: Name "ObjectProxy" already defined (possibly by an import)  [no-redef]
src/wrapt/__wrapt__.py:24: error: Name "CallableObjectProxy" already defined (possibly by an import)  [no-redef]
src/wrapt/__wrapt__.py:24: error: Name "PartialCallableObjectProxy" already defined (possibly by an import)  [no-redef]
src/wrapt/__wrapt__.py:24: error: Name "FunctionWrapper" already defined (possibly by an import)  [no-redef]
src/wrapt/__wrapt__.py:24: error: Name "BoundFunctionWrapper" already defined (possibly by an import)  [no-redef]
src/wrapt/__wrapt__.py:24: error: Name "_FunctionWrapperBase" already defined (possibly by an import)  [no-redef]
src/wrapt/importer.py:19: error: Need type annotation for "_post_import_hooks" (hint: "_post_import_hooks: dict[<type>, <type>] = ...")  [var-annotated]
src/wrapt/decorators.py:521: error: "Callable[[Any], Any]" has no attribute "_synchronized_meta_lock"  [attr-defined]
Found 12 errors in 5 files (checked 8 source files)

Anyway, am spending what time I can on wrapt today, so will progress things as much as I can.

[GrahamDumpleton]

Looks like pytest-mypy-plugins only supports Python 3.9+ but wrapt is targeting 3.8+ still.

[GrahamDumpleton]

I have made a lot of changes to wrapt now including:

• Started transition to uv for environment and package management.
• Switched to a Justfile to drive stuff, including running of tests.
• Drive pytest via uv so can easily test all Python versions.
• Created a py.typed of partial and empty stub for __init__.pyi.
• Add just tests to run mypy for Python 3.9+.
• Added the pytest-mypy-plugin for Python 3.9+.
• Set flags to ignore any mypy errors which aren't really errors or couldn't change.
• Added some type hints where mypy was complaining.
• Added a TESTING.md file which talks about running tests etc.

So I think were are in a good spot to finally start making some progress. No mypy errors remaining now at this point.

I still need to look at how this mypy plugin for pytest works and that YAML file in the PR to understand how they work and are used, so that is next.

[GrahamDumpleton]

FWIW. Am not seeing that test_mypy.yaml appearing in tox tests run under GitHub actions for this PR. So I will have to try it locally and see of any extra config needed.

[GrahamDumpleton]

Ahh, it was running. Stupid browser search.

[GrahamDumpleton]

Would this all be more precisely defined as:

from typing import Callable, TypeVar, Any, Tuple, Dict, Union

WrappedFunction = TypeVar("WrappedFunction", bound=Callable)

# Standard signature for wrappers
StandardWrapper = Callable[[WrappedFunction, Any, Tuple[Any, ...], Dict[str, Any]], Any]

# Catch-all wrapper signature
CatchAllWrapper = Callable[..., Any]

# Union type to accept either wrapper style
WrapperFunction = Union[StandardWrapper, CatchAllWrapper]

def FunctionWrapper(
    wrapped: WrappedFunction, wrapper: WrapperFunction
) -> WrappedFunction: ...

The standard wrapper would be:

    def standard_wrapper(wrapped, instance, *args, **kwargs):
        pass

It would be rare, but someone may want to instead define this as:

    def catch_all_wrapper(*args, **kwargs):
        pass

Anyway, starting to understand what I need to do. Just working out easy way of maintaining the tests. It is a pain to have to add in the out section with exact line numbers.

[GrahamDumpleton]

The way that pytest-mypy-plugins uses a YAML file is a pain to maintain. A much simpler approach would be to have convention where files which have sample code in it would be called tests/mypy_function_wrapper.py.

Next you just run:

mypy --show-error-codes tests/mypy_function_wrapper.py > tests/mypy_function_wrapper.out

and then the pytest plugin could be set up to find files matching tests/mypy_*.py with corresponding .out file, run the mypy check on the Python code file and compare to the output file.

This combining stuff into a single YAML file is an extra step which just makes it annoying.

Don't understand why the overall pytest-mypy-plugins looks so complicated. Am going to look into what is involved in making a pytest plugin and see if I can't make something much simpler which makes it easier to maintain the tests.

[GrahamDumpleton]

Well that was easy to write my own simpler custom test handler. Well at least, easy for the AI Copilot. 🤣

[GrahamDumpleton]

At this point I have basically invalidated this PR and we can close it.

Have a close look at develop branch in GitHub for what I have done.

I have pushed up the modified FunctionWrapper type hints I suggested above, along with test framework additions for testing with mypy.

Although have added initial test code for FunctionWrapper in tests/mypy_function_wrapper.py, with expected output in tests/mypy_function_wrapper.out, I want to go back over that and perhaps add some more tests across multiple test files for FunctionWrapper. Still getting a feel for using mypy` to add tests, but making progress.

After that, next step would be to work out properly what type hints are needed and so work out which to tackle next. Maybe we create a new issue at a time to propose what type hints should be for certain things, iterate over it in discussion as need be and then create a PR. But then maybe I will get enthusiastic over the weekend and just work out a whole. 😁