[Type Hint] Unet Models #330
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
sidthekidder
force-pushed
the
type_hints_models
branch
from
63e4e44 to
a273710
Compare
src/diffusers/models/unet_2d.py
Outdated
| @@ -30,7 +30,7 @@ def __init__( | |||
| attention_head_dim=8, | |||
| norm_num_groups=32, | |||
| norm_eps=1e-5, | |||
| ): | |||
| ) -> None: | |||
There was a problem hiding this comment.
Nice None is the correct return type here according to https://stackoverflow.com/questions/46779046/correct-type-annotation-for-init (no @anton-l ?)
@sidthekidder, could you maybe also give all the init values some type hints? I've shown one example above :-)
There was a problem hiding this comment.
@patrickvonplaten yeah, but it has become more of a soft requirement for __init__, as it can only return None. E.g. in mypy: python/mypy#604
So no worries if we don't annotate them :)
There was a problem hiding this comment.
Yeah agree here think it's also cleaner to not have a -> None type
There was a problem hiding this comment.
Makes sense, no return annotation for the constructor is cleaner - removed None!
src/diffusers/models/unet_2d.py
Outdated
| @@ -30,7 +30,7 @@ def __init__( | |||
| attention_head_dim=8, | |||
| norm_num_groups=32, | |||
| norm_eps=1e-5, | |||
There was a problem hiding this comment.
| norm_eps=1e-5, | |
| norm_eps: float =1e-5, |
There was a problem hiding this comment.
Hello
From issue #287, I see that type hints are required for all the variables. So would you be adding the type hints for other variables as well? Or is this PR out of scope for all of them?
Here are some examples:
flip_sin_to_cos: bool=True,
down_block_types: tuple[str]=("DownBlock2D", "AttnDownBlock2D", "AttnDownBlock2D", "AttnDownBlock2D"),
block_out_channels: tuple[int]=(224, 448, 672, 896),
act_fn: str="silu"
There was a problem hiding this comment.
Thanks for the examples @vishnu-anirudh and @patrickvonplaten - I added type hints for other __init__ variables as well.
| @@ -30,7 +30,7 @@ def __init__( | |||
| norm_eps=1e-5, | |||
There was a problem hiding this comment.
| norm_eps=1e-5, | |
| norm_eps: float =1e-5, |
Could you give all those values some type hints as well? :-)
sidthekidder
force-pushed
the
type_hints_models
branch
from
90cfb04 to
45df73e
Compare
|
Super cool - thanks a lot @sidthekidder ❤️ |
* add void check * remove void, add types for params
* add void check * remove void, add types for params
Adds type hint to init in 1. and 2. for issue #287