Skip to content

fix type for TestCase.assertIn #2186

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

Merged
merged 2 commits into from
Jun 4, 2018
Merged

fix type for TestCase.assertIn #2186

merged 2 commits into from
Jun 4, 2018

Conversation

JelleZijlstra
Copy link
Member

@JelleZijlstra JelleZijlstra commented Jun 1, 2018

This function does essentially assert member in container, so we want a Container, not an Iterable.

This came up in https://github.com/ambv/black/pull/288/files/68e9d426a86edc187a3f58ea889a2002620f0de6..0bbee43d60dfc16d8bbdd0bbdaad754a2c8c7ec0#r192525658.

I also switched the types to Any from a TypeVar, because the way __contains__ and Container implemented the argument doesn't do much anyway: __contains__ always accepts object.

@JelleZijlstra JelleZijlstra changed the title [WIP] fix type for TestCase.assertIn fix type for TestCase.assertIn Jun 1, 2018
@zsol
Copy link
Contributor

zsol commented Jun 1, 2018

Based on this I think there should be overloads for this for sequences, dicts, sets, strings, bytes, etc. But maybe that's overdoing it 😆

@JelleZijlstra
Copy link
Member Author

That does sound like overdoing it. :)

But from your link I now think we should allow both Iterable and Container, since x in y also works if y is an Iterable. This makes me think the Container ABC is not all that useful.

@ambv ambv merged commit eef0b1d into master Jun 4, 2018
@JelleZijlstra JelleZijlstra deleted the JelleZijlstra-patch-2 branch June 4, 2018 22:38
yedpodtrzitko pushed a commit to yedpodtrzitko/typeshed that referenced this pull request Jan 23, 2019
* fix type for TestCase.assertIn

This does essentially `assert member in container`, so we want a `Container`, not an `Iterable`.

This came up in https://github.com/ambv/black/pull/288/files/68e9d426a86edc187a3f58ea889a2002620f0de6..0bbee43d60dfc16d8bbdd0bbdaad754a2c8c7ec0#r192525658.

* use any for assertIn
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.

3 participants