Annotate depends and accept_arguments decorators

[gandhis1]

It took some tinkering but I finally have this working. Addresses #958

@MarcSkovMadsen @philippjfr

Results:

>> mypy param/_utils.py --follow-imports=silent
Success: no issues found in 1 source file

>> mypy param/depends.py --follow-imports=silent
param/depends.py:142: error: Item "str" of "Parameter | str" has no attribute "owner"  [union-attr]
param/depends.py:144: error: Item "str" of "Parameter | str" has no attribute "owner"  [union-attr]
param/depends.py:144: error: Item "str" of "Parameter | str" has no attribute "name"  [union-attr]

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 16 lines in your changes missing coverage. Please review.

Project coverage is 86.45%. Comparing base (31f5717) to head (06112c9).
Report is 1 commits behind head on main.

Files	Patch %	Lines
param/depends.py	42.85%	12 Missing ⚠️
param/_utils.py	63.63%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #962      +/-   ##
==========================================
- Coverage   86.70%   86.45%   -0.25%     
==========================================
  Files          10       10              
  Lines        5151     5176      +25     
==========================================
+ Hits         4466     4475       +9     
- Misses        685      701      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[gandhis1]

Without the /, Mypy complains:

param/depends.py:40: error: Argument 1 to "accept_arguments" has incompatible type "Callable[[CallableT@depends, VarArg(Parameter | str), DefaultNamedArg(bool, 'watch'), DefaultNamedArg(bool, 'on_init'), KwArg(Parameter)], Callable[[CallableT@depends], DependsFunc[CallableT@depends]]]"; expected "Callable[[CallableT, VarArg(Parameter | str), DefaultNamedArg(bool, 'watch'), DefaultNamedArg(bool, 'on_init'), KwArg(Parameter)], Callable[[CallableT], DependsFunc[CallableT]]]"  [arg-type]
param/depends.py:40: note: This is likely because "depends" has named arguments: "func". Consider marking them positional-only

This looks like a Mypy bug; variadic arguments via * should necessarily mean that func is positional-only. However, adding in / I believe makes no meaningful difference at runtime, so I just went ahead and did that.

[gandhis1]

The intention with these overloads is to prevent usage that violates the validation that depends applies internally at runtime, for example, this fails:

@depends("test", foobar=Parameter())
def test(a: int, b: str) -> None:
    print("foobar")

param/depends.py:154: error: No overload variant of "depends" matches argument types "str", "Parameter"  [call-overload]
param/depends.py:154: note: Possible overload variants:
param/depends.py:154: note:     def depends(*dependencies: str, watch: bool = ..., on_init: bool = ...) -> Callable[[CallableT], DependsFunc[CallableT]]
param/depends.py:154: note:     def depends(*dependencies: Parameter, watch: bool = ..., on_init: bool = ..., **kw: Parameter) -> Callable[[CallableT], DependsFunc[CallableT]]

While this works:

@depends(Parameter(), foobar=Parameter())
def test(a: int, b: str) -> None:
    print("foobar")

[gandhis1]

Is it intentional that this project is still on Python 3.8 while panel is on python 3.10? The python 3.8 build is failing here

Edit: Made it Python 3.8-compatible regardless

[hoxbro]

Is it intentional that this project is still on Python 3.8 while panel is on python 3.10?

Yes. Panel depends on projects that have dropped support for Python 3.9. Param is more of a fundamental tool, so we don't want to drop Python versions before it is necessary. See HEP1

[hoxbro]

Why do you use underscore here? Is this the norm?

It is "hidden" behind TYPE_CHECKING, so it will be private for users. It also adds extra overhead to reading the type hinting itself.

[gandhis1]

It is typical for typeshed, for example: https://github.com/python/typeshed/blob/main/stdlib/urllib/request.pyi#L51

It is "hidden" behind TYPE_CHECKING, so it will be private for users.

Not if they are also using TYPE_CHECKING:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from param._utils import P

And regardless of TYPE_CHECKING it still becomes visible in IDE auto-complete, interspersed with symbols that are part of the public API (as opposed to being namespaced with an underscore prefix):

(Granted, underscore is just a convention, they could just as well import it anyway.)

That said, I don't really care either way, so I'll just remove the underscores

[hoxbro]

Thank you for the clarification.

Whatever the norm should be done, and if you say it is making them private, then so be it.

If you want to avoid repeating complex types, you are welcome to create a _typing.py file, though it does not need to be in this PR.

Do you have a preferred way to test type hints? If possible, I would like your help in setting it up.

And like Philipp said, I also appreciate your work on these PRs! 💯

[gandhis1]

Do you have a preferred way to test type hints?

For these PRs I generally am just running mypy path/to/file/changed.py --follow-imports=silent and making sure that the errors after are a strict subset of the errors that existed before. I also just create some dummy code that uses the decorator to see whether it works as expected.

For something more automated - and I'm mostly thinking in the context of panel - we could set up a Mypy run that's part of CI. We could start with some subset of the examples folder that we can get zero issues on, then extend to all the examples, and after we reach 100% no issues, we can switch to Mypy strict and repeat. The point would be to prioritize the public API - which examples would encompass - over the internal library code.

I figure examples would work a lot better than me manually creating functions in test code and applying @param.depends etc. on it.

Only after that would I try to Mypy panel or param internal code as that is a much bigger lift and could require some actual code changes to be type-compatible, etc. So we'd probably want to run the examples CI check with --follow-imports=silent so that it doesn't prematurely start checking internal code.

[hoxbro]

I have opened a PR for Panel here holoviz/panel#7151

[philippjfr]

Really appreciate this, thanks for all your efforts so far!

[gandhis1]

Mergeable? Build seems to be flaky, assuming there's no good reason for it to fail on 3.12 windows and pass on 3.12 mac