feat: Add typing for function_decorator

[last-partizan]

I think, it is finally working as expected. Please take a look.

I don't know how to add tests for typing properly, but i've added test file, which could be checked with mypy and pyright.

[smarie]

Thanks a lot @last-partizan ! I'll have to fix #24 so that gh actions is able to build your PR.

I'll do my best but my agenda is a bit crowded these weeks. Please pardon me if it takes a bit longer than usual

[smarie]

Hi @last-partizan , I updated the layout and tests so that the project is in good shape again. I made a 1.4.9 release. Can you now please pull from main (renamed master branch) to refresh the PR ?

Note: do not hesitate to create a new PR if that's easier.

[last-partizan]

Done!

[smarie]

Just being curious here: isn't there a way to assert typing-related things in pytest ? We can even consider using typing-inspect or another such package if it makes sense..

Alternately we could add a mypy step in the nox file, scanning precisely this test file ?

[last-partizan]

I remember few days ago seeing article about testing typing in python packages, but couldn't find it at the moment.

We cannot just run mypy or pyright in CI, becouse it will exit with errors, and we want to be sure that it fails where we expect it to fail.

I think, our best shot is to run mypy and pyright using subprocess in pytest, and compare it's output with what we're expecting. I could try to do this in next PR or in this, what do you think?

But before that i want to think a little about other options, and maybe see what others are doing.

[last-partizan]

As for typing-inspect, i don't think it could be used in this case. Sure it could check something, but i'd want to test exact cases.

[smarie]

I see

[last-partizan]

Actually, i found something.

django-types are running pyright in subprocess, and parsing it's messages

https://github.com/sbdchd/django-types/blob/main/tests/pyright/base.py

and, mypy could be called from pytest using it's api:

https://mypy.readthedocs.io/en/stable/extending_mypy.html

I tried using mypy, but strumbled upon a bug which crashes mypy. Now i'm trying to fix it or make minimal example to report bug in mypy.

I think, merging this could actually crash mypy for someone who uses it.

python/mypy#8645

They already have similar issues with other code.

[last-partizan]

Oh, nevermind. It was crashing becouse it has cached some data from previous version in my environment. It's working fine. Just doesn't supports those ParamSpecs properly yet.

So, i think, if we want to add type testing, we could use pyright just like django-types. (But maybe that's for another PR)

[smarie]

Great ! Ok, let's keep this for another PR.

[smarie]

Thanks @last-partizan ! There seem to be some errors in the build, I did not have time to look yet. I also made a few comments.

[last-partizan]

Thanks @last-partizan ! There seem to be some errors in the build, I did not have time to look yet. I also made a few comments.

Ouch, of course pytest tried to run test test_typing, i'm renaming it into "typing_cases.py" to avoid this.

update: I left the same name, and now pytest can verify that type checking errors are real errors. I need some time to make it work and pass the tests.

[last-partizan]

ERR:     def __call__(self, func: F) -> F:
ERR:                            ^
ERR: SyntaxError: invalid syntax

https://github.com/last-partizan/python-decopatch/runs/5161137573?check_suite_focus=true

Oh, we can't use this syntax with 2.7. I could covert this to comments syntax, and it will work, probably. But we should consider dropping 2.7 (and maybe 3.5). What do you think?

[smarie]

Thanks a lot @last-partizan !

Well I would prefer not to drop python 2 now "just" for a typing-related topic - I was rather expecting that this PR would ship in a minor or patch version update. Dropping python 2 may be done in a future major release (including refactoring for example) rather. But frankly I rather hope that the python language will evolve in a way where we do not need decopatch anymore :)

For now, what about using a .pyi file to put the @overload definitions ? You basically just have to create the file, move all of your code into it (no need for the try except block, it can be 3.9+), and the typing tool should recognize it. See example here: https://github.com/smarie/python-pyfields/blob/main/pyfields/init_makers.pyi

That way the PR will not modify main.py but just create a main.pyi (+ a test or pseudo-test).

Concerning the test, if there is no easy way to automate, then we can stick to the simple manual test you proposed - at least that is something.

I'll be away with limited access for about 10 days, but I'll definitely come back to this when I'm back. Thanks again for your time !

[last-partizan]

.pyi is sounds pretty good, i'll try it!

[last-partizan]

Everything ready!

[smarie]

Ideally we should do the same for class_decorator and for decorator, don't you think ?

Note that _Decorator might need to change to support decorating Types too. Or, even more ideally, we would have a _FuncDecorator for function_decorator, a _ClassDecorator for class_decorator and a _Decorator = Union[_FuncDecorator, _ClassDecorator] for decorator.

Would it make sense ?

Note: this may be for another PR

Suggested change

	def class_decorator(
	enable_stack_introspection: bool = ...,
	custom_disambiguator: Callable[[Any], FirstArgDisambiguation] = ...,
	flat_mode_decorated_name: Optional[str] = ...,
	): ...
	# @class_decorator() is called without options or parenthesis
	@overload
	def class_decorator(
	enable_stack_introspection: Callable[P, Any],
	custom_disambiguator: Callable[[Any], FirstArgDisambiguation] = ...,
	flat_mode_decorated_name: Optional[str] = ...,
	) -> _Decorator[P]: ...
	# @class_decorator() is called with options or parenthesis.
	@overload
	def class_decorator(
	enable_stack_introspection: bool = ...,
	custom_disambiguator: Callable[[Any], FirstArgDisambiguation] = ...,
	flat_mode_decorated_name: Optional[str] = ...,
	) -> Callable[[Callable[P, Any]], _Decorator[P]]: ...

[last-partizan]

Yeah, that makes sense. I could try to do it in next PR.

[smarie]

perfect

[smarie]

Thanks a lot @last-partizan ! Two items left open

• the comment about doing the same for class_decorator and decorator: maybe in this PR or in another one, I let you decide, let me know (please add corresponding tests if you do so in this PR).
• the comment about using pyright in tests, just like django-types: I am ok to open it in another issue+PR

[last-partizan]

I'd better add tests using pyright in another PR.

[smarie]

Perfect, thanks @last-partizan ! Let me know if you wish to take a stab at the other PRs before a release, or after.

[last-partizan]

After release would be great!

[smarie]

Deeply sorry for not taking note of your response sooner. I just made a 1.4.10 tag, it should ship the associated pypi version in a few minutes