Add signature for attr.evolve #14526

ikonst · 2023-01-26T04:16:36Z

Validate attr.evolve calls to specify correct arguments and types.

The implementation makes it so that at every point where attr.evolve is called, the signature is modified to expect the attrs class' initializer's arguments (but so that they're all kw-only and optional).

Notes:

Added class dict: pass to some fixtures files since our attrs type stubs now have **kwargs and that triggers a builtin.dict lookup in dozens of attrs tests.
Looking up the type of the 1st argument with ctx.api.expr_checker.accept(inst_arg) which is a hack since it's not part of the plugin API. This is a compromise for due to Adjusting the signature of a function on the basis of caller types in plugin #10216.

Fixes #14525.

TeamSpen210 · 2023-01-26T17:49:36Z

mypy/plugins/default.py

+    ) -> Callable[[FunctionSigContext], FunctionLike] | None:
+        from mypy.plugins import attrs
+
+        if fullname in ("attr.evolve", "attr.assoc"):


There’s also attrs.evolve() - maybe that’s just an alias and will work, but it should be tested. Perhaps these should be moved to a constant with the others at the start of the attrs plug-in module also?

Yep, yep. All still WIP.

…olve

ikonst · 2023-01-27T17:16:31Z

mypy-primer identified an error in home-assistant:

new = self.areas[area_id] = attr.evolve(old, **new_values)

Typing new_values as a typed dict with the appropriate fields can address that. The misleading error is mentioned in #14073.

ikonst · 2023-01-27T19:53:13Z

test-data/unit/fine-grained.test

+2: <b>, <b[wildcard]>, __main__
+3: <b>, <b[wildcard]>, __main__, a


So.... the addition of dict in fixtures/list.pyi now causes __annotations__ to exist:

mypy/mypy/semanal.py

Lines 636 to 639 in bac9e77

elif name == "__annotations__":

sym = self.lookup_qualified("__builtins__.dict", Context(), suppress_errors=True)

if not sym:

continue

which then makes __annotations__ one of the names in the diff (reasonably...):

mypy/mypy/server/update.py

Lines 781 to 795 in a08388c

if item.count(".") <= package_nesting_level + 1 and item.split(".")[-1] not in (

"__builtins__",

"__file__",

"__name__",

"__package__",

"__doc__",

):

# Activate catch-all wildcard trigger for top-level module changes (used for

# "from m import *"). This also gets triggered by changes to module-private

# entries, but as these unneeded dependencies only result in extra processing,

# it's a minor problem.

#

# TODO: Some __* names cause mistriggers. Fix the underlying issue instead of

# special casing them here.

diff.add(id + WILDCARD_TAG)

I've created #14547 to discuss this more.

Addressed in #14550 and #14575.

…olve

ikonst · 2023-03-02T21:48:16Z

@AlexWaygood can label with attrs? :)

ikonst · 2023-03-02T23:00:51Z

🤔 Curious why the mypy_primer diff here is gone:
https://github.com/home-assistant/core/blob/8968ed1c47314db4b9ee7eeb302c92c4e71915a3/homeassistant/helpers/area_registry.py#L219

ikonst · 2023-03-03T15:58:45Z

@JelleZijlstra would love to get some eyes on this (I was waiting for #14575 to merge to reduce the diff)

github-actions · 2023-03-03T16:43:01Z

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

JelleZijlstra · 2023-03-03T18:12:24Z

I can take a look in the next few days.

This function is basically equivalent to dataclasses.replace, right? Would be nice if we can support that function in a similar way (in a separate PR).

JelleZijlstra

Looks good, just one minor point.

JelleZijlstra · 2023-03-06T00:05:03Z

mypy/types.py

@@ -1775,7 +1775,7 @@ def copy_modified(
        self: CT,
        arg_types: Bogus[Sequence[Type]] = _dummy,
        arg_kinds: Bogus[list[ArgKind]] = _dummy,
-        arg_names: Bogus[list[str | None]] = _dummy,
+        arg_names: Bogus[Sequence[str | None]] = _dummy,


What is the reason for this change? I'd prefer to avoid touching the mypy core in this PR.

Because of list's invariance — otherwise, this fails since it passes list[str]:

arg_names=["inst"] + attrs_init_type.arg_names[1:],

CallableType.__init__ accepts arg_names: Sequence[str | None] but I guess CallableType.copy_modified was not updated in tandem.

Broke out into #14840 — can merge then update this branch.

ikonst · 2023-03-06T14:39:34Z

mypy/plugins/attrs.py

+    # <hack>
+    assert isinstance(ctx.api, TypeChecker)
+    inst_type = ctx.api.expr_checker.accept(inst_arg)
+    # </hack>


@JelleZijlstra now that we got this merged 😅 what would be the right place to discuss whether this should be promoted to be formal plugin API?

Open an issue, I suppose.

👉 #14845

LukasKrocek · 2023-04-06T12:37:12Z

hey, I have tried to run this in my project, doen't work well when evolving generic classes

class A(Generic[T]):
    field: T

a = A("1")

evolve(a, field="2") ```

# 2 errors are raised:
#     Argument 1 to "evolve" of "A[str]" has incompatible type "A[str]"; expected "A[T]"  [arg-type]
#     Argument "field" to "evolve" of "A[str]" has incompatible type "int"; expected "T"  [arg-type]

ikonst · 2023-04-06T14:55:46Z

@LukasKrocek this was just mentioned in the review for #14849 — addressed in c005895, but here it would actually be simpler

ikonst · 2023-04-07T02:37:08Z

Generics should be addressed in #15016.

Validate `dataclassses.replace` actual arguments to match the fields: - Unlike `__init__`, the arguments are always named. - All arguments are optional except for `InitVar`s without a default value. The tricks: - We're looking up type of the first positional argument ("obj") through private API. See #10216, #14845. - We're preparing the signature of "replace" (for that specific dataclass) during the dataclass transformation and storing it in a "private" class attribute `__mypy-replace` (obviously not part of PEP-557 but contains a hyphen so should not conflict with any future valid identifier). Stashing the signature into the symbol table allows it to be passed across phases and cached across invocations. The stashed signature lacks the first argument, which we prepend at function signature hook time, since it depends on the type that `replace` is called on. Based on #14526 but actually simpler. Partially addresses #5152. # Remaining tasks - [x] handle generic dataclasses - [x] avoid data class transforms - [x] fine-grained mode tests --------- Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

ikonst force-pushed the 2023-01-25-attrs-evolve branch from e47698c to 042aa94 Compare January 26, 2023 04:18

wip

cd832cc

ikonst force-pushed the 2023-01-25-attrs-evolve branch from 042aa94 to cd832cc Compare January 26, 2023 04:25

This comment has been minimized.

Sign in to view

ikonst added 2 commits January 26, 2023 11:06

more resilient

64a9e58

hmpf

c145514

This comment has been minimized.

Sign in to view

TeamSpen210 reviewed Jan 26, 2023

View reviewed changes

wip

3477d2d

This comment has been minimized.

Sign in to view

rely on ExpressionChecker.accept :|

056ac66

ikonst mentioned this pull request Jan 27, 2023

attrs plugin should support attrs.evolve #14525

Closed

ikonst added 2 commits January 27, 2023 11:09

Merge remote-tracking branch 'origin/master' into 2023-01-25-attrs-ev…

8b7f3ca

…olve

handle Any, more tests

d976202

This comment has been minimized.

Sign in to view

make type-checker happy

1bdd315

This comment has been minimized.

Sign in to view

nits

4c8138d

ikonst marked this pull request as ready for review January 27, 2023 17:41

This comment has been minimized.

Sign in to view

teach fixtures about dict for kwargs

46026fe

This comment has been minimized.

Sign in to view

fix random test failures

2316b06

ikonst commented Jan 27, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

ikonst added 2 commits January 27, 2023 16:25

style

5148e5f

improve tests

cf756d1

ikonst force-pushed the 2023-01-25-attrs-evolve branch from 7188141 to cf756d1 Compare January 27, 2023 21:31

another test

34c8c94

Merge remote-tracking branch 'origin/master' into 2023-01-25-attrs-ev…

babf91f

…olve

ikonst force-pushed the 2023-01-25-attrs-evolve branch from ab8d33b to babf91f Compare January 30, 2023 16:51

break up evolve tests

5443d51

This comment has been minimized.

Sign in to view

Merge remote-tracking branch 'origin/master' into 2023-01-25-attrs-ev…

5271233

…olve

AlexWaygood added the topic-attrs label Mar 2, 2023

This comment has been minimized.

Sign in to view

clarify signature

bfcb366

This comment has been minimized.

Sign in to view

Merge branch 'master' into 2023-01-25-attrs-evolve

0b79cc1

JelleZijlstra self-requested a review March 3, 2023 18:13

JelleZijlstra approved these changes Mar 6, 2023

View reviewed changes

JelleZijlstra merged commit bbc9cce into python:master Mar 6, 2023

ikonst commented Mar 6, 2023

View reviewed changes

ikonst deleted the 2023-01-25-attrs-evolve branch March 6, 2023 15:48

ikonst mentioned this pull request Apr 7, 2023

Fix attrs.evolve with generics #15016

Closed

dermotduffy mentioned this pull request Apr 11, 2023

Merge the v0.12 branch now that Frigate v0.12 has been released blakeblackshear/frigate-hass-integration#458

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add signature for attr.evolve #14526

Add signature for attr.evolve #14526

ikonst commented Jan 26, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

TeamSpen210 Jan 26, 2023

ikonst Jan 26, 2023

ikonst Jan 27, 2023

This comment has been minimized.

This comment has been minimized.

ikonst commented Jan 27, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ikonst Jan 27, 2023 •

edited

Loading

ikonst Mar 6, 2023

This comment has been minimized.

This comment has been minimized.

ikonst commented Mar 2, 2023

This comment has been minimized.

ikonst commented Mar 2, 2023

ikonst commented Mar 3, 2023

This comment has been minimized.

github-actions bot commented Mar 3, 2023

JelleZijlstra commented Mar 3, 2023

JelleZijlstra left a comment

JelleZijlstra Mar 6, 2023

ikonst Mar 6, 2023 •

edited

Loading

ikonst Mar 6, 2023

ikonst Mar 6, 2023

JelleZijlstra Mar 6, 2023

ikonst Mar 6, 2023

LukasKrocek commented Apr 6, 2023 •

edited

Loading

ikonst commented Apr 6, 2023

ikonst commented Apr 7, 2023

		2: <b>, <b[wildcard]>, __main__
		3: <b>, <b[wildcard]>, __main__, a

	elif name == "__annotations__":
	sym = self.lookup_qualified("__builtins__.dict", Context(), suppress_errors=True)
	if not sym:
	continue

	if item.count(".") <= package_nesting_level + 1 and item.split(".")[-1] not in (
	"__builtins__",
	"__file__",
	"__name__",
	"__package__",
	"__doc__",
	):
	# Activate catch-all wildcard trigger for top-level module changes (used for
	# "from m import *"). This also gets triggered by changes to module-private
	# entries, but as these unneeded dependencies only result in extra processing,
	# it's a minor problem.
	#
	# TODO: Some __* names cause mistriggers. Fix the underlying issue instead of
	# special casing them here.
	diff.add(id + WILDCARD_TAG)

Add signature for attr.evolve #14526

Add signature for attr.evolve #14526

Conversation

ikonst commented Jan 26, 2023 • edited Loading

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

ikonst commented Jan 27, 2023 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ikonst Jan 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

ikonst commented Mar 2, 2023

This comment has been minimized.

ikonst commented Mar 2, 2023

ikonst commented Mar 3, 2023

This comment has been minimized.

github-actions bot commented Mar 3, 2023

JelleZijlstra commented Mar 3, 2023

JelleZijlstra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikonst Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukasKrocek commented Apr 6, 2023 • edited Loading

ikonst commented Apr 6, 2023

ikonst commented Apr 7, 2023

ikonst commented Jan 26, 2023 •

edited

Loading

ikonst commented Jan 27, 2023 •

edited

Loading

ikonst Jan 27, 2023 •

edited

Loading

ikonst Mar 6, 2023 •

edited

Loading

LukasKrocek commented Apr 6, 2023 •

edited

Loading