httpcore icon indicating copy to clipboard operation
httpcore copied to clipboard

Published 20 hours ago • verified publisher icon encode

Enable Mypy `--strict` internally

Open michaeloliverx opened this issue 2 years ago • 12 comments

I think being compatible with mypy --strict benefits end users who are also using it, if https://github.com/encode/httpcore/pull/513 is merged then there isn't much work left to be compatible with strict mode.

See also https://github.com/encode/httpcore/issues/512

michaeloliverx avatar Mar 01 '22 09:03 michaeloliverx

What do @encode/maintainers think of this?

michaeloliverx avatar Mar 30 '22 13:03 michaeloliverx

Would we do it only for the codebase or also for tests? The benefit of doing it for tests is that you get to asses what the experience would be like for a user in strict mode.

adriangb avatar Mar 30 '22 13:03 adriangb

I see you did both in your PR. I'm in favor of it 😁

adriangb avatar Mar 30 '22 14:03 adriangb

Based on #524 I'd probably be somewhat against it - looks like extra noise & fluff from my perspective.

tomchristie avatar Mar 30 '22 14:03 tomchristie

Would we do it only for the codebase or also for tests? The benefit of doing it for tests is that you get to asses what the experience would be like for a user in strict mode.

+1 for tests

I didn't type tests before I read this article https://sethmlarson.dev/blog/tests-arent-enough-case-study-after-adding-types-to-urllib3#type-your-tests which outlined the same benefit.

michaeloliverx avatar Mar 30 '22 14:03 michaeloliverx

Based on #524 I'd probably be somewhat against it - looks like extra noise & fluff from my perspective.

I agree its quite a bit of noise.. another option would be to do it incrementally as files are changed?

michaeloliverx avatar Mar 30 '22 14:03 michaeloliverx

Based on #524 I'd probably be somewhat against it - looks like extra noise & fluff from my perspective.

Are there particular changes in the PR that you would rather see addressed separately, @tomchristie?

  1. Narrowing # type: ignore to a particular class of error
  2. Correcting incomplete dict and list types
  3. Using type aliases on values with domain significance (HeadersAsMapping versus dict)
  4. Casting types brought in from the h2 package

2. is the most important for the purpose of the PR. Each correction means one less error showing up to the user in VS Code and the like.

RA80533 avatar Apr 24 '22 21:04 RA80533

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar May 31 '22 03:05 stale[bot]

adding typing in anyio found a large number of type level bugs, I'm in favor of this change - so unmarking stale

graingert avatar Jun 19 '22 14:06 graingert

I'm okay with us doing this, or at least with us moving towards it incrementally. @michaeloliverx's work on #524 showed that it's quite a big change footprint. I'd be happy to see PRs that either deal with changes on a one-module-at-a-time or one-kind-of-fix-at-a-time basis.

tomchristie avatar Oct 07 '22 19:10 tomchristie

I think we probably want to turn on --strict one file at a time right?

adriangb avatar Oct 07 '22 19:10 adriangb

Yeah that sort of thing. It'd probably be okay to make the fixes incrementally, without necessarily ratcheting them in using the CI. We could improve them bit by bit and only hard-enforce it once we get there.

tomchristie avatar Oct 07 '22 19:10 tomchristie

See https://github.com/encode/httpx/issues/2436 for how I think we should approach this.

tomchristie avatar Nov 17 '22 11:11 tomchristie