Skip to content

Allow tuple as input of query parameters. - #1426

Merged
florimondmanca merged 6 commits into
encode:masterfrom
SarunasAzna:master
Dec 12, 2020
Merged

Allow tuple as input of query parameters.#1426
florimondmanca merged 6 commits into
encode:masterfrom
SarunasAzna:master

Conversation

@SarunasAzna

Copy link
Copy Markdown
Contributor

In the documentation it is stated that params can be dict, string or two
tuples. This allows to used two tuples. Previously it was possible to
use only tuple inside a list.

In the documentation it is stated that params can be dict, string or two
tuples. This allows to used two tuples. Previously it was possible to
use only tuple inside a list.
@StephenBrown2

Copy link
Copy Markdown
Contributor

The documentation is not wrong, it states specifically that "a list of two-tuples" is allowed.

params - (optional) Query parameters to include in request URLs, as a string, dictionary, or list of two-tuples.

While a tuple of two-tuples should be fine, I'd like to hear the other maintainers thoughts on it. If accepted, you will also need to update the documentation to include "list or tuple of two-tuples." everywhere query params are mentioned, and adjust the QueryParamTypes signature:

httpx/httpx/_types.py

Lines 34 to 41 in 354c4ca

QueryParamTypes=Union[
"QueryParams",
Mapping[str, Union[PrimitiveData, Sequence[PrimitiveData]]],
List[Tuple[str, PrimitiveData]],
str,
bytes,
None,
]
to include Tuple[Tuple[str, PrimitiveData]], which should fix the currently failing mypy tests.

Comment threadhttpx/_models.py Outdated
Comment threadhttpx/_models.py Outdated
@SarunasAzna

Copy link
Copy Markdown
ContributorAuthor

@StephenBrown2 good point about the docs! So actually I think it is important to mention how I found this thing - at the moment we are moving some API agents from requests to httpx and the test with two tuples failed.

@StephenBrown2

Copy link
Copy Markdown
Contributor

Ah yes, that is an important note! We certainly want to maintain requests compatibility where possible, so go ahead and make the changes I suggested and the others can weigh in once that's done.

@florimondmancaflorimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 LGTM as this is — "sequence" in the docs, (list, tuple) in implementation.

@florimondmancaflorimondmanca added the enhancement New feature or request label Dec 12, 2020
@florimondmanca
florimondmanca merged commit 8696405 into encode:masterDec 12, 2020
@florimondmanca

Copy link
Copy Markdown
Contributor

@SarunasAzna Thanks!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Development

Successfully merging this pull request may close these issues.

3 participants

@SarunasAzna@StephenBrown2@florimondmanca