getsentry / responses

A utility for mocking out the Python Requests library.
Apache License 2.0
4.14k stars 354 forks source link

types-pyyaml is not needed at runtime #667

Closed asottile-sentry closed 1 year ago

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (771477f) 100.00% compared to head (1962ce4) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #667 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 9 9 Lines 2968 2968 ========================================= Hits 2968 2968 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

beliaev-maksim commented 1 year ago

Then people who install the package will have issues in imported module, no?

asottile-sentry commented 1 year ago

Then people who install the package will have issues in imported module, no?

types-pyyaml provides only .pyi files -- these aren't importable

beliaev-maksim commented 1 year ago

Yes, but when they run mypy on their side, all types should be present. Then either users will need to add it to their dependencies or we provide it.

Or does mypy work differently?

asottile-sentry commented 1 year ago

mypy will prompt them to install types-pyyaml if needed -- we could add an extra here for mypy but it shouldn't get installed for general users of responses that may not be using mypy

beliaev-maksim commented 1 year ago

If I remember right, there were complaints in the past about type checks and we added all the deps. So, it was deliberate choice

@markstory should we stick or change?

I recommend to keep

asottile-sentry commented 1 year ago

If I remember right, there were complaints in the past about type checks and we added all the deps. So, it was deliberate choice

@markstory should we stick or change?

I recommend to keep

if that were the case then types-requests would also be a hard dependency -- since that also will produce a message from mypy:

$ mypy responses/ | grep requests | head -1
responses/matchers.py:14:1: error: Library stubs not installed for "requests"  [import]

but typically typing-only deps are not part of the library deps

markstory commented 1 year ago

Users of responses that are using the recorder to create yaml files generally wouldn't have to directly interact with pyyaml. I wasn't able to find any issues asking for type packages to be installed, however I did find one asking for typing packages to not be part of the install_requires (#595).