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

Pyright: QueryParameterType does not support None for dictionary values #193

Closed
cardisnotvalid opened this issue Dec 27, 2024 · 1 comment · Fixed by #195
Closed

Pyright: QueryParameterType does not support None for dictionary values #193

cardisnotvalid opened this issue Dec 27, 2024 · 1 comment · Fixed by #195

Comments

@cardisnotvalid
Copy link

cardisnotvalid commented Dec 27, 2024

Expected Result

QueryParameterType should allow None for dictionary values to match how _encode_params handles them during encoding.

Actual Result

QueryParameterType rejects None in dict values, causing type errors, while _encode_params accepts it at runtime.

Reproduction Steps

import niquests
from typing import Dict, Optional

params: Dict[str, Optional[str]] = {"key1": "value1", "key2": None}
encoded = niquests.PreparedRequest._encode_params(params)
print(encoded) # key1=value1
$ pyright test.py
d:\Programming\test.py
  d:\Programming\test.py:14:51 - error: Argument of type "dict[str, str | None]" cannot be assigned to parameter "data" of type "QueryParameterType | BodyFormType | IO[Unknown]" in function "_encode_params"
    Type "dict[str, str | None]" is not assignable to type "QueryParameterType | BodyFormType | IO[Unknown]"
      "dict[str, str | None]" is not assignable to "List[Tuple[str, str | List[str]]]"
      "dict[str, str | None]" is not assignable to "Mapping[str, str | List[str]]"
        Type parameter "_VT_co@Mapping" is covariant, but "str | None" is not a subtype of "str | List[str]"
          Type "str | None" is not assignable to type "str | List[str]"
            Type "None" is not assignable to type "str | List[str]"
      "dict[str, str | None]" is not assignable to "bytes"
      "dict[str, str | None]" is not assignable to "str"
    ... (reportArgumentType)
1 error, 0 warnings, 0 informations

models.py:817-823
_encode_params skips None values during encoding.

System Information

$ python -m niquests.help
{
  "charset_normalizer": {
    "version": "3.4.0"
  },
  "http1": {
    "h11": "0.14.0"
  },
  "http2": {
    "jh2": "5.0.4"
  },
  "http3": {
    "enabled": true,
    "qh3": "1.2.1"
  },
  "idna": {
    "version": "3.10"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.11.9"
  },
  "niquests": {
    "version": "3.11.4"
  },
  "ocsp": {
    "enabled": true
  },
  "platform": {
    "release": "10",
    "system": "Windows"
  },
  "system_ssl": {
    "name": "OpenSSL 3.0.13 30 Jan 2024",
    "version": "300000d0"
  },
  "urllib3.future": {
    "cohabitation_version": "2.3.0",
    "version": "2.12.904"
  },
  "wassima": {
    "certifi_fallback": false,
    "enabled": true,
    "version": "1.1.5"
  },
  "websocket": {
    "enabled": false,
    "wsproto": null
  }
}
@Ousret
Copy link
Member

Ousret commented Dec 29, 2024

You are correct. Requests historically accepted None values and discarded them silently afterward.
We improved the typing definition in #195
It look like a continuation of #118 / #119

Regards,

@Ousret Ousret mentioned this issue Jan 1, 2025
Ousret added a commit that referenced this issue Jan 1, 2025
3.12.0 (2025-01-01)
-------------------

**Fixed**
- Restoring the state of `AsyncSession` through pickle.
- Typing definition for query parameter not accepting `None` as values.
(#193)
- Overload incorrect definition for `AsyncSession::get`. (#192)

**Added**
- Support for `PathLike` objects for `verify` parameter when passing a
ca bundle path. (#194)
- Caching and restoring OCSP state through pickling `Session` or
`AsyncSession`.
- Caching and restoring QUIC known compatible hosts through pickling
`Session` or `AsyncSession`.
- Shortcut convenient access to `Retry` and `Timeout` configuration
objects in top-level import.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants