param icon indicating copy to clipboard operation
param copied to clipboard

Implement descriptor factory typing system

Open philippjfr opened this issue 5 months ago • 17 comments

Supersedes https://github.com/holoviz/param/pull/1065 and https://github.com/holoviz/param/pull/985

After reviewing the other typing PRs and reading in a bit more depth on what is possible, including traitlets source code (which @maximlt pointed me to), this PR aims to provide an implementation of typing using Generics plus factory style typing overloads that handle polymorphic types that depend on a dynamic value (like allow_None).

This approach seems to play well with pylance so correctly infers types inside VSCode, without having the user have to manually type things, even if multiple type variants exist for a particular Parameter type. I've not gotten very far down the road of typing each parameter type but at least for the String type this works well, i.e. this works:

import param
from typing import reveal_type

class Test(param.Parameterized):

    string = param.String(default='foo')

    string_or_none = param.String(default=None, allow_None=True)

    implicit_string_or_none = param.String(default=None)

t = Test()

reveal_type(t.string) # type: str
reveal_type(t.string_or_none) # type: str | None
reveal_type(t.implicit_string_or_none) # type: str | None

philippjfr avatar Jul 15 '25 15:07 philippjfr

Reviewing this, I guess it looks fine. The if t.TYPE_CHECKING: bits are gnarly, but if that's what it takes to get type checking, then sure!

What's the status of this? Are you looking for specific feedback? Does it supersede the previous attempts entirely? What does it take to get it in?

jbednar avatar Jul 25 '25 03:07 jbednar

Thanks for creating this.

How do you plan to get a proper init signature? I don't see this in this PR?

MarcSkovMadsen avatar Jul 25 '25 04:07 MarcSkovMadsen

Reviewing this, I guess it looks fine. The if t.TYPE_CHECKING: bits are gnarly, but if that's what it takes to get type checking, then sure!

Indeed pretty gnarly, but it is what it is.

What's the status of this?

Planning to add tests and then start typing each parameter until the tests pass, or I have to loosen the tests if achieving proper typing isn't actually possible. For example I started on ObjectSelector and am dubious that I can correctly type the value as Literal[objects].

Are you looking for specific feedback?

Not really, unless someone can propose yet another cleaner or better alternative.

Does it supersede the previous attempts entirely?

Yes!

How do you plan to get a proper init signature?

You mean Parameter or Parameterized signatures?

philippjfr avatar Jul 25 '25 15:07 philippjfr

Parameterized init signatures. That was the starting point for my PR analysis.

MarcSkovMadsen avatar Jul 25 '25 19:07 MarcSkovMadsen

Another source of inspiration is mssgspec. It's quite readable. They use @dataclass_transform too to get init right. See https://github.com/jcrist/msgspec/blob/main/msgspec/init.pyi.

MarcSkovMadsen avatar Jul 26 '25 08:07 MarcSkovMadsen

For example I started on ObjectSelector and am dubious that I can correctly type the value as Literal[objects].

Yep also dubious this will be possible. I'd probably be fine with adding an item_type slot to Selector and have that reflected as a typing annotation.

Parameterized init signatures. That was the starting point for my PR analysis.

I'm fine with that being done in a separate PR. So I guess that means this PR doesn't strictly supersede https://github.com/holoviz/param/pull/1065 (though as far as I could see https://github.com/holoviz/param/pull/1065 was not implementing @dataclass_transform).

maximlt avatar Jul 27 '25 05:07 maximlt

#1065 is using @dataclass_transform. Its the whole point that we need an approach that supports both attribute types and init signatures. They need to be compatible. Can't create one feature without at least proof that other feature will be possible.

MarcSkovMadsen avatar Jul 27 '25 09:07 MarcSkovMadsen

#1065 is using @dataclass_transform. Its the whole point that we need an approach that supports both attribute types and init signatures. They need to be compatible. Can't create one feature without at least proof that other feature will be possible.

Didn't work for me there https://github.com/holoviz/param/pull/1065#discussion_r2203176175. How did you get it to work? Maybe I'm doing something wrong.

maximlt avatar Jul 27 '25 10:07 maximlt

Yeah also tried #1065 and had no luck getting it to work for __init__ signatures.

philippjfr avatar Jul 27 '25 14:07 philippjfr

It works for some attributes. Those that are specifically typed. And those that are created by overloaded 'parameter' function.

Cannot/ will never work for Parameter classes unless mypy/ pylance add support for Param or we provide plugin.

image

MarcSkovMadsen avatar Jul 27 '25 15:07 MarcSkovMadsen

what’s the if t.TYPE_CHECKING for?

if it’s there in order to keep Sphinx from picking up all the alternative signatures, I think it might be possible to use this: https://github.com/tox-dev/sphinx-autodoc-typehints/blob/e9db2aaee73c6dfe466cde83ea502dc4c33c2fb6/src/sphinx_autodoc_typehints/patches.py#L20-L35

flying-sheep avatar Aug 11 '25 14:08 flying-sheep

Okay, I have gotten much further along here and though there is still plenty of work to fix runtime behavior I've broken, finish typing all of param and define appropriate overloads for all Parameter types, I now have an understanding of dataclass_transform, which I did not have before.

The Problem

Overall dataclass_transform simply will not play well with param as it is defined today. RHS (right-hand-side) type definitions never get bound to the LHS, i.e. in other words a Parameter definition even if fully and correctly typed will never bind the type to the dataclass-like object (i.e. the Parameterized). This means that whatever we do you would still have to repeat yourself, e.g.:

class MyClass(Parameterized):

    string_param: str = String()

the type definition is always needed, even though the String descriptor fully expresses its type. Worse yet, because descriptors classes cannot lie about their type you actually have to type it as:

class MyClass(Parameterized):

    string_param: String[str] = String()

And finally, we cannot declare all Parameter types as valid dataclass field specifiers because each Parameter type explicitly has to be listed in the dataclass_transform(field_specifiers=...) and we do not have all Parameter classes defined when we define ParameterizedMetaclass (which the dataclass_transform decorator is applied to).

An Alternative

Okay, so it's now clear that we will never get automatically generated __init__ signatures for standard Parameterized classes. But what could we do? My proposal would be to introduce a new TypedParameterized (we can workshop the name) along with a param factory helper. In this proposal the definition could look something like this:

class MyClass(TypedParameterized):

    string: str
    explicit_str: str = param(str, doc="Explicitly defined parameter type definition")
    explicit_str_param: str = param(param.String, doc="Explicitly defined parameter instance definition")
  • Typed attributes are automatically upgraded to parameter definitions
  • The param wrapper allows creating parameter definitions from types and Parameter classes

Optionally we can then explore whether we can leverage and upgrade existing dataclasses and pydantic models by using TypedParameterized as a mixin.

philippjfr avatar Aug 27 '25 12:08 philippjfr

Also I realize much of this just recaps what @MarcSkovMadsen has already laid out above. I just wanted to record all the limitations for myself. As was also indicated previously, this PR will aim to do two things:

  1. Ensure that the whole param codebase passes mypy/pyright type checks
  2. Add overloads to all Parameter types to ensure the Parameterized attributes are typed appropriately (where possible).

Separately we should agree for a new proposal for a TypedParameterized that follows the dataclass standards and will therefore provide a __init__ signature.

philippjfr avatar Aug 27 '25 12:08 philippjfr

TypedParameterized -> Paramtyped ? Typedparams?

A mixin makes sense. Making a conversion function from one to the other (including as a decorator) would also make sense; not sure of the pros and cons.

What about making this new class be a dataclass already?

See https://docs.pydantic.dev/latest/concepts/dataclasses/ for how Pydantic handles this.

jbednar avatar Aug 28 '25 19:08 jbednar

I do like the idea of a @parameterize decorator for dataclasses.

philippjfr avatar Aug 29 '25 20:08 philippjfr

I think just getting the type inference in is already super valuable.

Typing for code like this will start working, which is a huge part of all Holo* code:

def do_sth_with_dim(dim: Dimension):
    name = dim.name  # now known to be str!
    ...

currently a lot of it looks like this instead:

grafik

flying-sheep avatar Sep 25 '25 14:09 flying-sheep

Totally agree, hoping I can wrap this up pretty soon. Once that's done I can begin working on a proposal for a TypedParameterized.

philippjfr avatar Sep 25 '25 16:09 philippjfr