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

[dataclass_transform] support implicit default for "init" parameter in field specifiers #14870

Closed

Conversation

wesleywright
Copy link
Collaborator

@wesleywright wesleywright commented Mar 10, 2023

This note from PEP 681 was missed in the initial implementation of field specifiers:

If unspecified, init defaults to True. Field specifier functions can use overloads that implicitly specify the value of init using a literal bool value type (Literal[False] or Literal[True]).

This commit adds support for reading a default from the declared type of the init parameter if possible. Otherwise, it continues to use the typical default of True.

if not isinstance(call, CallExpr):
return True

specifier_type = _get_callee_type(call)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm reusing this existing helper from mypy.plugins.common, but unfortunately it doesn't truly support overloads:

    if isinstance(callee_node, (Var, SYMBOL_FUNCBASE_TYPES)) and callee_node.type:
        callee_node_type = get_proper_type(callee_node.type)
        if isinstance(callee_node_type, Overloaded):
            # We take the last overload.
            return callee_node_type.items[-1]
        elif isinstance(callee_node_type, CallableType):
            return callee_node_type

I'm not sure if there's an existing method that can resolve the appropriate overload or if some new plumbing will need to be added to MyPy.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can't generally resolve overloads in a dataclass plugin, since semantic analysis is not complete yet, and certain type operations aren't strictly speaking available yet. We could perhaps do a best-effort resolution, but I'm not sure if it's worth the effort. Do you know of any use case where this is needed? Perhaps we can come up with a simplified implementation that is good enough for common use cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@JukkaL I'm not sure if anything is using it in the wild, thought he PEP calls it out with an explicit example: https://peps.python.org/pep-0681/#field-specifier-parameters We've discussed this some in #14293 (comment), so I can try asking there if there any concrete use cases to be aware of.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@wesleywright wesleywright changed the title [dataclass_transform] support implicit default for "init" parameter i… [dataclass_transform] support implicit default for "init" parameter in field specifiers Mar 13, 2023
@JukkaL
Copy link
Collaborator

JukkaL commented Apr 4, 2023

I'll try finishing this up so that we can include it in the 1.2 release.

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 5, 2023

Closing in favor of #15010, which builds on top of this and adds overload resolution.

@JukkaL JukkaL closed this Apr 5, 2023
JukkaL added a commit that referenced this pull request Apr 5, 2023
…n field specifiers (#15010)

(Basic functionality was implemented by @wesleywright in #14870. I added
overload resolution.)

This note from PEP 681 was missed in the initial implementation of field 
specifiers:

> If unspecified, init defaults to True. Field specifier functions can
use overloads that implicitly specify the value of init using a literal
bool value type (Literal[False] or Literal[True]).

This commit adds support for reading a default from the declared type of
the `init` parameter if possible. Otherwise, it continues to use the
typical default of `True`.

The implementation was non-trivial, since regular overload resolution
can't be used in the dataclass plugin, which is applied before type
checking. As a workaround, I added a simple overload resolution helper
that should be enough to support typical use cases. It doesn't do full
overload resolution using types, but it knows about `None`,
`Literal[True]` and `Literal[False]` and a few other things.

---------

Co-authored-by: Wesley Collin Wright <wesleyw@dropbox.com>
JukkaL added a commit that referenced this pull request Apr 5, 2023
…n field specifiers (#15010)

(Basic functionality was implemented by @wesleywright in #14870. I added
overload resolution.)

This note from PEP 681 was missed in the initial implementation of field 
specifiers:

> If unspecified, init defaults to True. Field specifier functions can
use overloads that implicitly specify the value of init using a literal
bool value type (Literal[False] or Literal[True]).

This commit adds support for reading a default from the declared type of
the `init` parameter if possible. Otherwise, it continues to use the
typical default of `True`.

The implementation was non-trivial, since regular overload resolution
can't be used in the dataclass plugin, which is applied before type
checking. As a workaround, I added a simple overload resolution helper
that should be enough to support typical use cases. It doesn't do full
overload resolution using types, but it knows about `None`,
`Literal[True]` and `Literal[False]` and a few other things.

---------

Co-authored-by: Wesley Collin Wright <wesleyw@dropbox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants