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

Use a JSON return type for requests.Response.json #9335

Closed
Viicos opened this issue Dec 5, 2022 · 4 comments
Closed

Use a JSON return type for requests.Response.json #9335

Viicos opened this issue Dec 5, 2022 · 4 comments

Comments

@Viicos
Copy link
Contributor

Viicos commented Dec 5, 2022

As stated here, recursive type aliases are supported by most type checkers, and a JSON type can now be properly defined. I'd like to know if changing the return type of the json method to this could be feasible.

def json(
self,
*,
cls: type[JSONDecoder] | None = ...,
object_hook: Callable[[dict[Any, Any]], Any] | None = ...,
parse_float: Callable[[str], Any] | None = ...,
parse_int: Callable[[str], Any] | None = ...,
parse_constant: Callable[[str], Any] | None = ...,
object_pairs_hook: Callable[[list[tuple[Any, Any]]], Any] | None = ...,
**kwds: Any,
) -> Any: ...

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Dec 5, 2022

Unfortunately, no. As I state in the comment you link:

As always, if you return a union type, your callers will have to assert the structure of their JSON in order to type check. If this an issue, return e.g. dict[str, Any] or Any or cast or type-ignore or just bite the bullet and assert.

Typeshed typically avoids union return types in cases where callers typically know their structure, since we prefer to avoid false positives over false negatives. You may be interested in python/typing#1096

@Viicos
Copy link
Contributor Author

Viicos commented Dec 6, 2022

Ok good to know that we are trying to avoid union return types! So for instance when making an API wrapper using requests, one could define the following method:

class MyClient:
    ...
    def get_ressource(self) -> dict[str, Any]:
        return cast(dict[str, Any], requests.get("http://api.example.com"))

If I know that my ressource will have the type dict[str, Any], could this be a viable solution?
As omitting the cast call will end up with a mypy warning (using --strict): Returning Any from function declared to return "Dict[str, Any]" [no-any-return]

Thanks in advance

@hauntsaninja
Copy link
Collaborator

Yes, that could work, but I'd do it slightly differently:

class MyClient:
    def get_resource(self) -> dict[str, Any]:
        ret: dict[str, Any] = requests.get("http://api.example.com").json()
        return ret

This avoids the cast, which is as unsafe as # type: ignore (and more unsafe than # type: ignore[no-any-return])... in particular, the cast covers up a real problem in your example, which is that you forgot .json() ;-)

That is, possible solutions are:

  • assign to a variable with a known type before returning
  • use # type: ignore[no-any-return] with the error code
  • use mypy --strict --no-warn-return-any (e.g. if you use a lot of untyped APIs)
  • finally, you can use cast or # type: ignore without the error code, but this could cover up other errors like forgetting to add .json()

@Viicos
Copy link
Contributor Author

Viicos commented Dec 6, 2022

Great catch indeed .json() was missing. I like the solution to assign to a variable before returning, in my opinion this is more readable than using cast. Thanks for the clear explanation! Closing.

@Viicos Viicos closed this as completed Dec 6, 2022
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

No branches or pull requests

2 participants