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

Response parameters type hints #178

Merged
merged 2 commits into from
Jun 9, 2021
Merged

Response parameters type hints #178

merged 2 commits into from
Jun 9, 2021

Conversation

OldSneerJaw
Copy link
Contributor

@OldSneerJaw OldSneerJaw commented May 30, 2021

Related to #173.

Makes improvements to the type hints of response parameters in both MockerCore and Adapter:

Also, as per @jamielennox's suggestion, I added "single star" parameters from PEP 3102 to indicate where the list of keyword-only arguments begin in both MockerCore and Adapter.

Makes improvements to the type hints of response parameters in both `MockerCore` and `Adapter`:
- Added explicit parameters for `reason`, `cookies`, `json`, `content`, `body`, `raw` and `exc` as per the documentation at https://requests-mock.readthedocs.io/en/latest/response.html#registering-responses
- Updated the `text` parameter to support static and dynamic values as per https://requests-mock.readthedocs.io/en/latest/response.html#dynamic-response
- Ensured that the "new" `json`, `content` and `body` parameters support both static and dynamic values, just like the `text` parameter
@jamielennox
Copy link
Owner

It does appear that all the dynamic typing that was done for the API makes getting type annotations right for this pretty challenging :)

Out of interest - it doesn't appear here that having _Context and _RequestObjectProxy as private makes any difference? At one point someone had asked to make them public as it would simplify some IDE things.

text: Union[str, Callable[[_RequestObjectProxy, _Context], str]] = ...,
content: Union[bytes, Callable[[_RequestObjectProxy, _Context], bytes]] = ...,
body: Union[IOBase, Callable[[_RequestObjectProxy, _Context], IOBase]] = ...,
raw: HTTPResponse = ...,
Copy link
Owner

Choose a reason for hiding this comment

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

interesting - i always thought this was callable as well. Guess noone has ever used that.

@jamielennox jamielennox merged commit 2d8be9e into jamielennox:master Jun 9, 2021
@OldSneerJaw
Copy link
Contributor Author

@jamielennox wrote:

It does appear that all the dynamic typing that was done for the API makes getting type annotations right for this pretty challenging :)

Out of interest - it doesn't appear here that having _Context and _RequestObjectProxy as private makes any difference? At one point someone had asked to make them public as it would simplify some IDE things.

Making the types public would help users of the API to annotate their own callback functions. As it is, a linter (e.g. pylint) might complain at the use of private types in the parameter annotations. For example, if I were to write my own text callback with type annotations, I might do something like this (borrowed and tweaked from the docs):

def text_callback(request: _RequestObjectProxy, context: _Context) -> str:
    context.status_code = 200
    context.headers['Test1'] = 'value1'
    return 'response'

Of course, changing the _RequestObjectProxy type name would technically be a breaking change for API clients that have already written any dynamic additional_matcher functions and annotated the parameters with _RequestObjectProxy. You could get around that issue by renaming _RequestObjectProxy to RequestObjectProxy and then adding a replacement _RequestObjectProxy that extends from it with an empty body, for example. On the bright side, I don't think it's an issue for _Context at this point since these new annotations haven't actually been released yet.

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 this pull request may close these issues.

None yet

2 participants