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

Invalid type definition for mpwriter.append #7741

Closed
1 task done
Tracked by #450
JP-Ellis opened this issue Oct 23, 2023 · 2 comments · Fixed by #8211
Closed
1 task done
Tracked by #450

Invalid type definition for mpwriter.append #7741

JP-Ellis opened this issue Oct 23, 2023 · 2 comments · Fixed by #8211
Labels

Comments

@JP-Ellis
Copy link

JP-Ellis commented Oct 23, 2023

Describe the bug

It would appear that the append method of the MultiPartWriter has an incorrect type definition:

def append(self, obj: Any, headers: Optional[MultiMapping[str]] = None) -> Payload:

Resulting in type checking tools to incorrectly errpr what (I believe) should be correct.

To Reproduce

The following test file generates the error in type checkers.

import aiohttp

with aiohttp.MultipartWriter() as mpwriter:
    mpwriter.append("Hello, World", {"Content-Type": "text/plain"})

which generates the following errors:

mypy test.py
test.py:4: error: Argument 2 to "append" of "MultipartWriter" has incompatible type "dict[str, str]"; expected "MultiMapping[str] | None"  [arg-type]
Found 1 error in 1 file (checked 1 source file)pyright test.py
/Users/joshua.ellis/src/pact-foundation/pact-python/tests/v3/test.py
  /Users/joshua.ellis/src/pact-foundation/pact-python/tests/v3/test.py:4:37 - error: Argument of type "dict[str, str]" cannot be assigned to parameter "headers" of type "MultiMapping[str] | None" in function "append"
    Type "dict[str, str]" cannot be assigned to type "MultiMapping[str] | None"
      "dict[str, str]" is incompatible with "MultiMapping[str]"
      Type cannot be assigned to type "None" (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations

Expected behavior

I expected that a regular Python dictionary be compatible with the header argument.

Logs/tracebacks

_see above_

Python Version

python --version
Python 3.11.6

aiohttp Version

python -m pip show aiohttp
Name: aiohttp
Version: 3.8.6
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Requires: aiosignal, async-timeout, attrs, charset-normalizer, frozenlist, multidict, yarl
Required-by:

multidict Version

python -m pip show multidict
Name: multidict
Version: 6.0.4
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Requires: 
Required-by: aiohttp, yarl

yarl Version

python -m pip show yarl
Name: yarl
Version: 1.9.2
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache-2.0
Requires: idna, multidict
Required-by: aiohttp

OS

uname -mprs
Darwin 22.6.0 arm64 arm

Related component

Client

Additional context

No response

Code of Conduct

  • I agree to follow the aio-libs Code of Conduct
@JP-Ellis JP-Ellis added the bug label Oct 23, 2023
@Dreamsorcerer
Copy link
Member

Yes, it appears the argument is always passed into CIMultiDict.update(), which should accept a Mapping. Feel free to make a PR to change the annotation (I suspect you'll need to tweak a couple of other annotations too).

@JP-Ellis
Copy link
Author

Great! I'll work on it later this week :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants