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

Type of default value for attrs.field() is revealed as Union[builtins.int, None] #16174

Open
gertvdijk opened this issue Sep 24, 2023 · 4 comments
Labels
bug mypy got something wrong topic-attrs

Comments

@gertvdijk
Copy link

gertvdijk commented Sep 24, 2023

Bug Report

Default (default=) value for attrs.field() type is revealed as Union[builtins.int, None] where the Union with None is unexpected. While this used to be revealed as Any, this shows up as a regression, bisected to 391ed853f (PR #15021, mypy master, unreleased).

To Reproduce

import attrs

@attrs.define()
class MyDataClass:
    some_int_field: int = attrs.field(default=123)

# Somewhere else in the project I would like to reference that default:
reveal = attrs.fields(MyDataClass).some_int_field.default
reveal_type(reveal)

assignment_to_int_fails: int = attrs.fields(MyDataClass).some_int_field.default

Expected Behavior

$ mypy --strict myfile.py
myfile.py:9:13: note: Revealed type is "builtins.int"
Success: no issues found in 1 source file

Actual Behavior

on master (tested 9edda9a):

$ mypy --strict --show-error-context --show-column-numbers myfile.py
myfile.py:9:13: note: Revealed type is "Union[builtins.int, None]"
myfile.py:11:32: error: Incompatible types in assignment (expression has type "int | None", variable has type "int")  [assignment]
Found 1 error in 1 file (checked 1 source file)

(my project fails, suspected mypy bug / imperfect type inference)

On 1.5.1:

$ mypy --strict myfile.py
myfile.py:9:13: note: Revealed type is "Any"
Success: no issues found in 1 source file

(my project passes, albeit without a proper type check)

Your Environment

  • Mypy version used: master (1.5.1 = OK, introduced with 391ed853f)
  • Mypy command-line flags: --strict --show-error-context --show-column-numbers
  • Mypy configuration options from mypy.ini (and other config files): n/a
  • Python version used: 3.11.5

Project to test on if you want (100% mypy-strict compatible on 1.5.1 and master otherwise): https://github.com/gertvdijk/PyKMP

@gertvdijk gertvdijk added the bug mypy got something wrong label Sep 24, 2023
@gertvdijk
Copy link
Author

Hi @Tinche! I noticed you sumitted a PR to revert some problematic behaviour introduced by that commit I bisected this to (#15393) and then @hauntsaninja proposed an alternative (#15688), so I wanted you guys to get this friendly ping.

Am I missing something or is there already some fix in the works for this? Thanks!

@Tinche
Copy link
Contributor

Tinche commented Sep 24, 2023

Hi,

this is the first I'm hearing of this so I don't think we have a fix in the works.

That said, I'm not sure this is easily fixable (except that the default type should actually be int | Literal[attrs.NOTHING] instead of int | None).

The type of fields(MyDataClass).some_int_field is attrs.Attribute[int], and there isn't enough information in that type signature for Mypy to determine whether the type of attrs.Attribute[int].default should be int or Literal[attrs.NOTHING] (i.e. whether the field has a default isn't knowable from the type).

I see two solutions:

  1. We change the Mypy plugin to make the type of Attribute.default be Any. This will remove your error but it's basically false type safety.
  2. Maybe we can change the signature of attrs.Attribute to be generic over two type variables; so in your case it'd be attrs.Attribute[int, Literal[123]]. (Or at least attrs.Attribute[int, int].) We aren't necessarily bound to the amount of type variables the actual attrs.Attribute class has; at runtime it has 0 currently. This approach is much more work but actually pushes the attrs typing boundary further in the right direction.

EDIT: The default value could also be an instance of attrs.Factory.

gertvdijk added a commit to gertvdijk/PyKMP that referenced this issue Oct 10, 2023
The upgrade of mypy from 1.5.1 to 1.6.0 introduces this error:

    src/pykmp/client.py: note: In member "send_request" of class
    "ClientCommunicator":

    src/pykmp/client.py:136:36: error: Incompatible default for argument
    "destination_address" (default has type "int | None", argument has
    type "int")  [assignment]

For more information see python/mypy#16174.

Because it's unclear if this typing complaint can be fixed properly,
this changes the destination address variable into a simple module level
constant for now.
@gertvdijk
Copy link
Author

gertvdijk commented Oct 10, 2023

Thanks for the thoughts and info. For now I've worked around the error and just made it a simple constant reused by both cases (see commit referenced above) and I think it's not a big deal.

@Tinche
Copy link
Contributor

Tinche commented Oct 10, 2023

I actually put in some work in the meantime to see if this is achievable. I think it's probably worthwhile but it'll need an attrs release as well, and I'm not sure how to coordinate exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-attrs
Projects
None yet
Development

No branches or pull requests

3 participants