Skip to content

[red-knot] Add support for calling type[…]#16597

Merged
sharkdp merged 3 commits intomainfrom
david/call-subclass-of-dynamic
Mar 10, 2025
Merged

[red-knot] Add support for calling type[…]#16597
sharkdp merged 3 commits intomainfrom
david/call-subclass-of-dynamic

Conversation

@sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Mar 10, 2025 •
edited
Loading

Summary

This fixes the non-diagnostics part of #15948, but is mostly an experiment to see if the new ecosystem checks work as intended (see failing mypy primer run here). I saw a few false positives when expressions of type type[<dynamic base>] were called, so this seemed rather easy to fix.

Test Plan

New Markdown tests.

Negative diff on the ecosystem checks:

zipp (https://github.com/jaraco/zipp)
- error: lint:call-non-callable
-    --> /tmp/mypy_primer/projects/zipp/zipp/__init__.py:393:16
-     |
- 392 |     def _next(self, at):
- 393 |         return self.__class__(self.root, at)
-     |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Object of type `type[Unknown]` is not callable
- 394 |
- 395 |     def is_dir(self):
-     |
- 
- Found 9 diagnostics
+ Found 8 diagnostics

arrow (https://github.com/arrow-py/arrow)
+     |
+     |
+ warning: lint:unused-ignore-comment
+    --> /tmp/mypy_primer/projects/arrow/arrow/arrow.py:576:66
+ 574 |                 values.append(1)
+ 575 |
+ 576 |             floor = self.__class__(*values, tzinfo=self.tzinfo)  # type: ignore[misc]
+     |                                                                  -------------------- Unused blanket `type: ignore` directive
+ 577 |
+ 578 |             if frame_absolute == "week":
- error: lint:call-non-callable
-     --> /tmp/mypy_primer/projects/arrow/arrow/arrow.py:1080:16
-      |
- 1078 |           dt = self._datetime.astimezone(tz)
- 1079 |
- 1080 |           return self.__class__(
-      |  ________________^
- 1081 | |             dt.year,
- 1082 | |             dt.month,
- 1083 | |             dt.day,
- 1084 | |             dt.hour,
- 1085 | |             dt.minute,
- 1086 | |             dt.second,
- 1087 | |             dt.microsecond,
- 1088 | |             dt.tzinfo,
- 1089 | |             fold=getattr(dt, "fold", 0),
- 1090 | |         )
-      | |_________^ Object of type `type[Unknown]` is not callable
- 1091 |
- 1092 |       # string output and formatting
-      |

black (https://github.com/psf/black)
- 
-     |
-     |
- error: lint:call-non-callable
-    --> /tmp/mypy_primer/projects/black/src/blib2to3/pgen2/grammar.py:135:15
- 133 |         Copy the grammar.
- 134 |         """
- 135 |         new = self.__class__()
-     |               ^^^^^^^^^^^^^^^^ Object of type `type[@Todo]` is not callable
- 136 |         for dict_attr in (
- 137 |             "symbol2number",
- Found 328 diagnostics
+ Found 327 diagnostics

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Mar 10, 2025
@sharkdp sharkdp changed the title [red-knot] Add support for calling type[<dynamic base>] [red-knot] Add support for calling type[<dynamic base>] Mar 10, 2025
@sharkdp sharkdp force-pushed the david/call-subclass-of-dynamic branch from fe532b6 to 40aaf41 Compare March 10, 2025 10:44
@sharkdp sharkdp marked this pull request as ready for review March 10, 2025 10:52
AlexWaygood
AlexWaygood reviewed Mar 10, 2025
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

The diff in the mypy_primer run looks great! It's unfortunate that it shows up as a "failed check" on the PR, though -- is that deliberate?

@sharkdp sharkdp force-pushed the david/call-subclass-of-dynamic branch from 40aaf41 to fc07a1c Compare March 10, 2025 12:04
@sharkdp
Copy link
Contributor Author

sharkdp commented Mar 10, 2025

It's unfortunate that it shows up as a "failed check" on the PR, though -- is that deliberate?

Signalling a non-empty diff as a 'failure' (that you need to look at) seems fine to me? Once we add commenting on PRs, we could potentially turn that into a check that always succeeds.

@sharkdp sharkdp changed the title [red-knot] Add support for calling type[<dynamic base>] [red-knot] Add support for calling type[…] Mar 10, 2025
@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 10, 2025

It's unfortunate that it shows up as a "failed check" on the PR, though -- is that deliberate?

Signalling a non-empty diff as a 'failure' (that you need to look at) seems fine to me? Once we add commenting on PRs, we could potentially turn that into a check that always succeeds.

hmm, I suspect this will be quite confusing for new contributors. Though having said that, contributors have also been confused over at typeshed by the mypy_primer comments. (A "sea of red" in the comment can be a good thing, since it shows that diagnostics are going away! But red usually connotes "bad" 😄)

sharkdp
sharkdp commented Mar 10, 2025
View reviewed changes
crates/red_knot_python_semantic/resources/mdtest/call/subclass_of.md
Comment on lines +23 to +26
# TODO: Those should all be errors
reveal_type(subclass_of_c("a")) # revealed: C
reveal_type(subclass_of_c()) # revealed: C
reveal_type(subclass_of_c(1, 2)) # revealed: C
Copy link
Contributor Author

@sharkdp sharkdp Mar 10, 2025

Choose a reason for hiding this comment

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

These will be fixed by #16512

AlexWaygood
AlexWaygood approved these changes Mar 10, 2025
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

I think overall I'd much prefer a "comment on the PR" mypy_primer check rather than have the check fail. But the changes this PR makes look great!

@sharkdp
Copy link
Contributor Author

sharkdp commented Mar 10, 2025

I think overall I'd much prefer a "comment on the PR" mypy_primer check rather than have the check fail

Ok, I'll look into it.

@sharkdp sharkdp merged commit c60e8a0 into main Mar 10, 2025
21 of 22 checks passed
@sharkdp sharkdp deleted the david/call-subclass-of-dynamic branch March 10, 2025 12:24
@sharkdp sharkdp mentioned this pull request Mar 10, 2025
Closed
dcreager added a commit that referenced this pull request Mar 10, 2025
@dcreager
Merge branch 'main' into dcreager/overloads
3aaf910 
* main:
  [red-knot] Add support for calling `type[…]` (#16597)
  Update migration guide with the new `ruff.configuration` (#16567)
  [red-knot] Add 'mypy_primer' workflow (#16554)
  Update Rust crate indoc to v2.0.6 (#16585)
  Update Rust crate syn to v2.0.100 (#16590)
  Update Rust crate thiserror to v2.0.12 (#16591)
  Update Rust crate serde_json to v1.0.140 (#16589)
  Update Rust crate quote to v1.0.39 (#16587)
  Update Rust crate serde to v1.0.219 (#16588)
  Update Rust crate proc-macro2 to v1.0.94 (#16586)
  Update Rust crate anyhow to v1.0.97 (#16584)
  Update dependency ruff to v0.9.10 (#16593)
  Update Rust crate unicode-ident to v1.0.18 (#16592)
  [red-knot] Do not ignore typeshed stubs for 'venv' module (#16596)
  [red-knot] Reduce Salsa lookups in `Type::find_name_in_mro` (#16582)
  Fix broken red-knot property tests (#16574)
  [red-knot] Consistent spelling of "metaclass" and "meta-type" (#16576)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

@carljm carljm Awaiting requested review from carljm

@MichaReiser MichaReiser Awaiting requested review from MichaReiser

Assignees

No one assigned

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sharkdp @AlexWaygood