Skip to content

Dict unpacking type checks don't consider **kwargs #17642

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

Closed
dinatamas opened this issue Aug 4, 2024 · 3 comments
Closed

Dict unpacking type checks don't consider **kwargs #17642

dinatamas opened this issue Aug 4, 2024 · 3 comments
Labels
bug mypy got something wrong topic-type-context Type context / bidirectional inference

Comments

@dinatamas
Copy link

dinatamas commented Aug 4, 2024

Bug Report:

In the below example (playground gist), mypy behaves differently with respect to **kwargs:

def func(x: bool = False, **kwargs):
    print(x, kwargs)

# This is accepted by mypy.
func(y="y")

# This is flagged by mypy.
data = {"y": "y"}
func(**data)

The error is Argument 1 to "func" has incompatible type "**dict[str, str]"; expected "bool" [arg-type]. This is incorrect because the function has **kwargs, so the dict / function call should be considered type correct.

Your Environment:

  • Mypy version used: latest (1.11.1)
  • Mypy command-line flags: -
  • Mypy configuration options from mypy.ini (and other config files): -
  • Python version used: 3.12
@153957
Copy link

153957 commented Oct 15, 2024

@JelleZijlstra JelleZijlstra added the topic-type-context Type context / bidirectional inference label Oct 15, 2024
@dinatamas
Copy link
Author

dinatamas commented Dec 1, 2024

I have overlooked that data may specify x.

def func(x: bool = False, **kwargs):
    print(x, kwargs)

data = {"x": "a"}
func(**data)  # warning

Since mypy has no way to know the content of the dictionary (unless it's a TypedDict or frozendict), it is correct to flag this function call.

If func only expects strings, mypy accepts this.

def func(x: str = "text", **kwargs):
    print(x, kwargs)

data = {"x": "a", "y": "b"}
func(**data)  # ok

It also accepts this if x is explicitly provided.

def func(x: bool = False, **kwargs):
    print(x, kwargs)

data = {"y": "b"}
func(x=True, **data)  # ok

@dinatamas
Copy link
Author

Hence mypy's behavior is correct and I'm closing this ticket.

HenryL27 added a commit to aryn-ai/sycamore that referenced this issue Mar 7, 2025
HenryL27 added a commit to aryn-ai/sycamore that referenced this issue Mar 20, 2025
* initial heirarchical document summarize implementation

Signed-off-by: Henry Lindeman <[email protected]>

* ruff

Signed-off-by: Henry Lindeman <[email protected]>

* make some tests work

Signed-off-by: Henry Lindeman <[email protected]>

* fix more tests

Signed-off-by: Henry Lindeman <[email protected]>

* mypy

Signed-off-by: Henry Lindeman <[email protected]>

* fix llm filter codegen

Signed-off-by: Henry Lindeman <[email protected]>

* put back collapsing summarizer

Signed-off-by: Henry Lindeman <[email protected]>

* fix names

Signed-off-by: Henry Lindeman <[email protected]>

* add docset summarizer parametrization

Signed-off-by: Henry Lindeman <[email protected]>

* add roundrobin summarizer

Signed-off-by: Henry Lindeman <[email protected]>

* mypy and ruff

Signed-off-by: Henry Lindeman <[email protected]>

* rename to RoundRobinOneshotDocumentSummarizer

Signed-off-by: Henry Lindeman <[email protected]>

* factor complicated common jinja logic to fragments

Signed-off-by: Henry Lindeman <[email protected]>

* add max tokens heirarchical summarizer

Signed-off-by: Henry Lindeman <[email protected]>

* ruff

Signed-off-by: Henry Lindeman <[email protected]>

* fix unit tests

Signed-off-by: Henry Lindeman <[email protected]>

* mypy

Signed-off-by: Henry Lindeman <[email protected]>

* add unit tests for summarizers

Signed-off-by: Henry Lindeman <[email protected]>

* a whole bunch of docstrings

Signed-off-by: Henry Lindeman <[email protected]>

* oops didn't mean to commit this

Signed-off-by: Henry Lindeman <[email protected]>

* move complex prompts to be next to the complex code that sets them up

Signed-off-by: Henry Lindeman <[email protected]>

* have summmarize_data take a summarizer instance rather than a summarizer class and all the ingredients needed to instantiate it duh

Signed-off-by: Henry Lindeman <[email protected]>

* fix unit tests

Signed-off-by: Henry Lindeman <[email protected]>

* inline get text macro since it's only used by one template

Signed-off-by: Henry Lindeman <[email protected]>

* remove collapse document summarizer

Signed-off-by: Henry Lindeman <[email protected]>

* apparently that lets me get rid of collapse and qasummarizer too, nice

Signed-off-by: Henry Lindeman <[email protected]>

* mypy + tests

Signed-off-by: Henry Lindeman <[email protected]>

* ruff

Signed-off-by: Henry Lindeman <[email protected]>

* initial easy comments

Signed-off-by: Henry Lindeman <[email protected]>

* redo MaxTokenHierarchySummarizer in more sensible way

Signed-off-by: Henry Lindeman <[email protected]>

* create SummarizeDocument Document subclass to make the hacky slurp-to-one-node-in-one-document slightly less hacky

Signed-off-by: Henry Lindeman <[email protected]>

* ruff

Signed-off-by: Henry Lindeman <[email protected]>

* mypy

Signed-off-by: Henry Lindeman <[email protected]>

* pytest

Signed-off-by: Henry Lindeman <[email protected]>

* more tests

Signed-off-by: Henry Lindeman <[email protected]>

* fix token counting for OneStepSummarizer

Signed-off-by: Henry Lindeman <[email protected]>

* drop a print statement

Signed-off-by: Henry Lindeman <[email protected]>

* ruff and mypy. mypy sucks. python/mypy#17642

Signed-off-by: Henry Lindeman <[email protected]>

* fix the integration tests that I think are my fault

Signed-off-by: Henry Lindeman <[email protected]>

* update summarize docs

Signed-off-by: Henry Lindeman <[email protected]>

* move summarizeDocument to summarize.py

Signed-off-by: Henry Lindeman <[email protected]>

* before summarize.py comments

Signed-off-by: Henry Lindeman <[email protected]>

* rewrite MultiStepDocumentSummarizer again

Signed-off-by: Henry Lindeman <[email protected]>

* prevent infinite summary loop

Signed-off-by: Henry Lindeman <[email protected]>

* mypy and ruff

Signed-off-by: Henry Lindeman <[email protected]>

* batch elements, not token counts

Signed-off-by: Henry Lindeman <[email protected]>

* mypy

Signed-off-by: Henry Lindeman <[email protected]>

* fix test_operations tests/bugs

Signed-off-by: Henry Lindeman <[email protected]>

* fix test_summarize bugs

Signed-off-by: Henry Lindeman <[email protected]>

* partition fields for OneStepSummarizer between documents and elements

Signed-off-by: Henry Lindeman <[email protected]>

* fix default summarizer constants to do something that makes sense

Signed-off-by: Henry Lindeman <[email protected]>

* drop leftover docstring args

Signed-off-by: Henry Lindeman <[email protected]>

---------

Signed-off-by: Henry Lindeman <[email protected]>
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-type-context Type context / bidirectional inference
Projects
None yet
Development

No branches or pull requests

3 participants