Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support dataclasses.InitVar for python>=3.8 #171

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

mixilchenko
Copy link
Contributor

No description provided.

@lebrice
Copy link
Owner

lebrice commented Oct 20, 2022

Wow, really interesting stuff @mixilchenko ! I'll do a proper review soon!

Copy link
Owner

@lebrice lebrice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really clean @mixilchenko , thanks for the PR!

I don't doubt that this will come in handy, but just out of curiosity, what use-case did you have in mind while implementing this?

Also, do you think that the init-var arguments should be grouped together on the command-line, or identified differently?
I think it might be a good idea to check how these fields appear on the command-line, and perhaps add a test of some sort so we can verify / keep track of that over the next python releases.

Thanks for doing this, and let me know what you think! :)

@mixilchenko
Copy link
Contributor Author

Speaking about dataclasses, InitVar always looked for me like some crutch when

  • you need postprocessing which can't be done without types breaking
  • default and default_factory are not enough to define default value
  • you need different types to accept and contain
  • ...

I am not so good with examples but here it is (https://mypy-play.net/?mypy=0.981&python=3.10 launchable)

from dataclasses import dataclass, field, InitVar
from functools import cached_property

@dataclass
class A1:
    value: int | str

    def __post_init__(self) -> None:
        self.value = int(self.value)
        
@dataclass
class A2:
    value: int
    
    def __post_init__(self) -> None:
        self.value = int(self.value)
        
@dataclass
class A3:
    arg: InitVar[int | str]
    value: int = field(init=False)
    
    def __post_init__(self, arg: int | str) -> None:
        self.value = int(arg)

class A4:
    arg: int | str
    
    @cached_property
    def value(self) -> int:
        return int(self.arg)

a1 = A1('2')
reveal_type(a1.value)  # Revealed type is "Union[builtins.int, builtins.str]"

a2 = A2('2')  # Argument 1 to "A2" has incompatible type "str"; expected "int"
reveal_type(a2.value)  # Revealed type is "builtins.int"

a3 = A3('2')
reveal_type(a3.value)  # Revealed type is "builtins.int"

a4 = A4('2')
reveal_type(a4.value)  # Revealed type is "builtins.int"

In that case, A3 is preferred over A4 for me because arg is not saved inside

Speaking about argument parser, I don't think that any additional marks are needed. InitVars do look like usual __init__ arguments and that seems totally reasonable for me.

Copy link
Owner

@lebrice lebrice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks interesting. I'm kind-of on the edge about how useful this will be, but I see no good reason not to include it.

Thanks again @mixilchenko !

@lebrice lebrice merged commit fa5e3f7 into lebrice:master Nov 2, 2022
@mixilchenko mixilchenko deleted the support-initvar branch November 2, 2022 20:38
lebrice pushed a commit that referenced this pull request Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants