Skip to content

Commit 69edb73

Browse files
fix: improve error handling for unknown formats in Querystring and required_args
1 parent 750354e commit 69edb73

4 files changed

Lines changed: 34 additions & 11 deletions

File tree

src/openai/_qs.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,12 @@ def _stringify_item(
7676
items: list[tuple[str, str]] = []
7777
nested_format = opts.nested_format
7878
for subkey, subvalue in value.items():
79+
if nested_format not in get_args(NestedFormat):
80+
raise NotImplementedError(
81+
f"Unknown nested_format value: {nested_format}, choose from {', '.join(get_args(NestedFormat))}"
82+
)
7983
items.extend(
8084
self._stringify_item(
81-
# TODO: error if unknown format
8285
f"{key}.{subkey}" if nested_format == "dots" else f"{key}[{subkey}]",
8386
subvalue,
8487
opts,

src/openai/_utils/_utils.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ def wrapper(*args: object, **kwargs: object) -> object:
276276
else:
277277
assert len(variants) > 0
278278

279-
# TODO: this error message is not deterministic
280-
missing = list(set(variants[0]) - given_params)
279+
# preserve declaration order for deterministic error messages
280+
missing = [param for param in variants[0] if param not in given_params]
281281
if len(missing) > 1:
282282
msg = f"Missing required arguments: {human_join([quote(arg) for arg in missing])}"
283283
else:

tests/test_qs.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,29 @@ def test_array_brackets(method: str) -> None:
7373
assert unquote(serialise({"a": {"b": [True, False, None, True]}})) == "a[b][]=true&a[b][]=false&a[b][]=true"
7474

7575

76+
@pytest.mark.parametrize("method", ["class", "function"])
77+
def test_array_indices(method: str) -> None:
78+
if method == "class":
79+
serialise = Querystring(array_format="indices").stringify
80+
else:
81+
serialise = partial(stringify, array_format="indices")
82+
83+
assert unquote(serialise({"in": ["foo", "bar"]})) == "in[0]=foo&in[1]=bar"
84+
assert unquote(serialise({"a": {"b": [True, False]}})) == "a[b][0]=true&a[b][1]=false"
85+
assert (
86+
unquote(serialise({"a": {"b": [True, False, None, True]}}))
87+
== "a[b][0]=true&a[b][1]=false&a[b][3]=true"
88+
)
89+
90+
7691
def test_unknown_array_format() -> None:
77-
with pytest.raises(NotImplementedError, match="Unknown array_format value: foo, choose from comma, repeat"):
92+
with pytest.raises(
93+
NotImplementedError,
94+
match="Unknown array_format value: foo, choose from comma, repeat, indices, brackets",
95+
):
7896
stringify({"a": ["foo", "bar"]}, array_format=cast(Any, "foo"))
97+
98+
99+
def test_unknown_nested_format() -> None:
100+
with pytest.raises(NotImplementedError, match="Unknown nested_format value: foo, choose from dots, brackets"):
101+
stringify({"a": {"b": "c"}}, nested_format=cast(Any, "foo"))

tests/test_required_args.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,16 @@ def foo(a: str = "", *, b: str = "", c: str = "") -> str | None:
4747

4848
assert foo(a="a", b="b", c="c") == "a b c"
4949

50-
error_message = r"Missing required arguments.*"
51-
52-
with pytest.raises(TypeError, match=error_message):
50+
with pytest.raises(TypeError, match=r"Missing required arguments: 'a', 'b' or 'c'"):
5351
foo()
5452

55-
with pytest.raises(TypeError, match=error_message):
53+
with pytest.raises(TypeError, match=r"Missing required arguments: 'b' or 'c'"):
5654
foo(a="a")
5755

58-
with pytest.raises(TypeError, match=error_message):
56+
with pytest.raises(TypeError, match=r"Missing required arguments: 'a' or 'c'"):
5957
foo(b="b")
6058

61-
with pytest.raises(TypeError, match=error_message):
59+
with pytest.raises(TypeError, match=r"Missing required arguments: 'a' or 'b'"):
6260
foo(c="c")
6361

6462
with pytest.raises(TypeError, match=r"Missing required argument: 'a'"):
@@ -78,7 +76,6 @@ def foo(*, a: str | None = None, b: str | None = None) -> str | None:
7876
assert foo(a=None) is None
7977
assert foo(b=None) is None
8078

81-
# TODO: this error message could probably be improved
8279
with pytest.raises(
8380
TypeError,
8481
match=r"Missing required arguments; Expected either \('a'\) or \('b'\) arguments to be given",

0 commit comments

Comments
 (0)