typeshed icon indicating copy to clipboard operation
typeshed copied to clipboard

Use Callable[..., Any] instead of Callable[..., object] in unittest

Open ShaneHarvey opened this issue 3 years ago • 10 comments

This is a follow up to resolve https://github.com/python/typeshed/issues/8372 by using Callable[..., Any] instead of Callable[..., object] in all apis. This is needed to workaround the mypy bug: https://github.com/python/mypy/issues/13220

Note I made these changes with this command:

grep -rl 'Callable.*, object' . | xargs sed -i '' 's/, object]/, Any]/g'

Edit: Updated to only change unittest.TestCase (and the DocTestCase subclass).

ShaneHarvey avatar Jul 25 '22 19:07 ShaneHarvey

If this approach seems reasonable, I'll work on fixing the "Any" is not defined errors (caused by Any not being imported in some files).

ShaneHarvey avatar Jul 25 '22 19:07 ShaneHarvey

Thanks for the PR. I'd like to know what other maintainers think here, but I'm somewhat unhappy with this change tbh, and I'd like to sink some time into trying to fix the mypy bug before we go ahead with something like this.

The fault is really with mypy here, not typeshed. Morally, the more correct type in basically all of these cases is Callable[<parameters>, object], since Callable is covariant in its return type, and since Any should generally only be used as an "escape hatch" or for when an argument's "true" type is currently inexpressible in the type system.

AlexWaygood avatar Jul 25 '22 20:07 AlexWaygood

Diff from mypy_primer, showing the effect of this PR on open source code:

discord.py (https://github.com/Rapptz/discord.py)
- discord/automod.py:219: error: Argument "data" to "from_data" of "AutoModTrigger" has incompatible type "object"; expected "Union[_AutoModerationTriggerMetadataKeyword, _AutoModerationTriggerMetadataKeywordPreset, Empty, None]"
- discord/app_commands/models.py:187: error: "object" has no attribute "__iter__"; maybe "__str__" or "__dir__"? (not iterable)
- discord/app_commands/models.py:194: error: No overload variant of "int" matches argument type "object"
- discord/app_commands/models.py:194: note: Possible overload variants:
- discord/app_commands/models.py:194: note:     def __new__(cls, Union[str, bytes, array[Any], mmap, _CData, PickleBuffer, SupportsInt, SupportsIndex, SupportsTrunc] = ...) -> int
- discord/app_commands/models.py:194: note:     def __new__(cls, Union[str, bytes], base: SupportsIndex) -> int
- discord/app_commands/models.py:201: error: Incompatible types in assignment (expression has type "object", variable has type "bool")
- discord/app_commands/models.py:202: error: Incompatible types in assignment (expression has type "object", variable has type "bool")
- discord/app_commands/models.py:791: error: Incompatible types in assignment (expression has type "object", variable has type "bool")
- discord/app_commands/models.py:792: error: Incompatible types in assignment (expression has type "object", variable has type "Union[int, float, None]")
- discord/app_commands/models.py:793: error: Incompatible types in assignment (expression has type "object", variable has type "Union[int, float, None]")
- discord/app_commands/models.py:794: error: Incompatible types in assignment (expression has type "object", variable has type "bool")
- discord/app_commands/models.py:795: error: "object" has no attribute "__iter__"; maybe "__str__" or "__dir__"? (not iterable)
- discord/app_commands/models.py:797: error: "object" has no attribute "__iter__"; maybe "__str__" or "__dir__"? (not iterable)
- discord/app_commands/models.py:884: error: "object" has no attribute "__iter__"; maybe "__str__" or "__dir__"? (not iterable)
- discord/scheduled_event.py:134: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[str]")
- discord/scheduled_event.py:140: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[str]")
- discord/scheduled_event.py:141: error: Incompatible types in assignment (expression has type "object", variable has type "int")
- discord/scheduled_event.py:144: error: Argument 1 to "store_user" of "ConnectionState" has incompatible type "object"; expected "Union[User, PartialUser]"
- discord/scheduled_event.py:146: error: No overload variant of "parse_time" matches argument type "object"
- discord/scheduled_event.py:146: note: Possible overload variants:
- discord/scheduled_event.py:146: note:     def parse_time(timestamp: None) -> None
- discord/scheduled_event.py:146: note:     def parse_time(timestamp: str) -> datetime
- discord/scheduled_event.py:146: note:     def parse_time(timestamp: Optional[str]) -> Optional[datetime]
- discord/scheduled_event.py:150: error: Argument 1 to "_unroll_metadata" of "ScheduledEvent" has incompatible type "object"; expected "Optional[EntityMetadata]"
- discord/scheduled_event.py:160: error: No overload variant of "int" matches argument type "object"
- discord/scheduled_event.py:160: note: Possible overload variants:
- discord/scheduled_event.py:160: note:     def __new__(cls, Union[str, bytes, array[Any], mmap, _CData, PickleBuffer, SupportsInt, SupportsIndex, SupportsTrunc] = ...) -> int
- discord/scheduled_event.py:160: note:     def __new__(cls, Union[str, bytes], base: SupportsIndex) -> int
- discord/partial_emoji.py:111: error: Argument "animated" to "PartialEmoji" has incompatible type "Union[object, Any]"; expected "bool"
- discord/appinfo.py:177: error: Incompatible types in assignment (expression has type "object", variable has type "int")
- discord/appinfo.py:178: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[str]")
- discord/invite.py:192: error: Incompatible types in assignment (expression has type "List[Literal['ANIMATED_ICON', 'BANNER', 'COMMERCE', 'COMMUNITY', 'DISCOVERABLE', 'FEATURABLE', 'INVITE_SPLASH', 'MEMBER_VERIFICATION_GATE_ENABLED', 'MONETIZATION_ENABLED', 'MORE_EMOJI', 'MORE_STICKERS', 'NEWS', 'PARTNERED', 'PREVIEW_ENABLED', 'PRIVATE_THREADS', 'ROLE_ICONS', 'TICKETED_EVENTS_ENABLED', 'VANITY_URL', 'VERIFIED', 'VIP_REGIONS', 'WELCOME_SCREEN_ENABLED']]", variable has type "List[str]")
+ discord/invite.py:192: error: Incompatible types in assignment (expression has type "Union[List[Literal['ANIMATED_ICON', 'BANNER', 'COMMERCE', 'COMMUNITY', 'DISCOVERABLE', 'FEATURABLE', 'INVITE_SPLASH', 'MEMBER_VERIFICATION_GATE_ENABLED', 'MONETIZATION_ENABLED', 'MORE_EMOJI', 'MORE_STICKERS', 'NEWS', 'PARTNERED', 'PREVIEW_ENABLED', 'PRIVATE_THREADS', 'ROLE_ICONS', 'TICKETED_EVENTS_ENABLED', 'VANITY_URL', 'VERIFIED', 'VIP_REGIONS', 'WELCOME_SCREEN_ENABLED']], List[<nothing>]]", variable has type "List[str]")
+ discord/invite.py:192: error: Argument 2 to "get" of "Mapping" has incompatible type "List[<nothing>]"; expected "Union[List[Literal['ANIMATED_ICON', 'BANNER', 'COMMERCE', 'COMMUNITY', 'DISCOVERABLE', 'FEATURABLE', 'INVITE_SPLASH', 'MEMBER_VERIFICATION_GATE_ENABLED', 'MONETIZATION_ENABLED', 'MORE_EMOJI', 'MORE_STICKERS', 'NEWS', 'PARTNERED', 'PREVIEW_ENABLED', 'PRIVATE_THREADS', 'ROLE_ICONS', 'TICKETED_EVENTS_ENABLED', 'VANITY_URL', 'VERIFIED', 'VIP_REGIONS', 'WELCOME_SCREEN_ENABLED']], List[str]]"
- discord/invite.py:392: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[bool]")
- discord/invite.py:397: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[int]")
- discord/invite.py:398: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[int]")
- discord/user.py:116: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[str]")
- discord/user.py:117: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[int]")
- discord/user.py:118: error: Incompatible types in assignment (expression has type "object", variable has type "int")
- discord/user.py:119: error: Incompatible types in assignment (expression has type "object", variable has type "bool")
- discord/user.py:120: error: Incompatible types in assignment (expression has type "object", variable has type "bool")
- discord/user.py:354: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[str]")
- discord/member.py:152: error: No overload variant of "parse_time" matches argument type "object"
- discord/member.py:152: note: Possible overload variants:
- discord/member.py:152: note:     def parse_time(timestamp: None) -> None
- discord/member.py:152: note:     def parse_time(timestamp: str) -> datetime
- discord/member.py:152: note:     def parse_time(timestamp: Optional[str]) -> Optional[datetime]
- discord/interactions.py:163: error: Incompatible types in assignment (expression has type "object", variable has type "Union[ChatInputApplicationCommandInteractionData, UserApplicationCommandInteractionData, MessageApplicationCommandInteractionData, ButtonMessageComponentInteractionData, SelectMessageComponentInteractionData, ModalSubmitInteractionData, None]")
- discord/guild.py:508: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[int]")
- discord/guild.py:509: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[int]")
- discord/guild.py:510: error: Incompatible types in assignment (expression has type "object", variable has type "bool")
- discord/channel.py:2582: error: "object" has no attribute "__iter__"; maybe "__str__" or "__dir__"? (not iterable)
- discord/abc.py:497: error: Need type annotation for "overridden"
- discord/abc.py:497: error: Argument 1 to "enumerate" has incompatible type "object"; expected "Iterable[<nothing>]"
- discord/webhook/async_.py:978: error: Argument "data" to "PartialWebhookChannel" has incompatible type "object"; expected "PartialChannel"
- discord/webhook/async_.py:980: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[PartialWebhookChannel]")
- discord/webhook/async_.py:984: error: Argument "data" to "PartialWebhookGuild" has incompatible type "object"; expected "SourceGuild"
- discord/webhook/async_.py:986: error: Incompatible types in assignment (expression has type "object", variable has type "Optional[PartialWebhookGuild]")
- discord/app_commands/tree.py:1047: error: Incompatible types in assignment (expression has type "object", variable has type "List[Union[_CommandGroupApplicationCommandInteractionDataOption, Union[_StringValueApplicationCommandInteractionDataOption, _IntegerValueApplicationCommandInteractionDataOption, _BooleanValueApplicationCommandInteractionDataOption, _SnowflakeValueApplicationCommandInteractionDataOption, _NumberValueApplicationCommandInteractionDataOption]]]")
- discord/app_commands/tree.py:1057: error: Incompatible types in assignment (expression has type "object", variable has type "List[Union[_CommandGroupApplicationCommandInteractionDataOption, Union[_StringValueApplicationCommandInteractionDataOption, _IntegerValueApplicationCommandInteractionDataOption, _BooleanValueApplicationCommandInteractionDataOption, _SnowflakeValueApplicationCommandInteractionDataOption, _NumberValueApplicationCommandInteractionDataOption]]]")
- discord/app_commands/tree.py:1089: error: Incompatible types in assignment (expression has type "object", variable has type "Union[str, int, None]")
- discord/ui/select.py:275: error: Incompatible types in assignment (expression has type "object", variable has type "List[str]")

rclip (https://github.com/yurijmikhalevich/rclip)
- rclip/db.py:70: error: Argument 1 to "update" of "MutableMapping" has incompatible type "NewImage"; expected "SupportsKeysAndGetItem[str, None]"
- rclip/db.py:70: note: Following member(s) of "NewImage" have conflicts:
- rclip/db.py:70: note:     Expected:
- rclip/db.py:70: note:         def __getitem__(self, str) -> None
- rclip/db.py:70: note:     Got:
- rclip/db.py:70: note:         def __getitem__(self, str) -> object

steam.py (https://github.com/Gobot1234/steam.py)
- steam/manifest.py:815: error: "object" has no attribute "get"  [attr-defined]
- steam/manifest.py:816: error: "object" has no attribute "get"  [attr-defined]
- steam/manifest.py:817: error: Value of type "object" is not indexable  [index]
- steam/manifest.py:817: error: Unsupported right operand type for in ("object")  [operator]
- steam/manifest.py:819: error: "object" has no attribute "get"  [attr-defined]
- steam/manifest.py:820: error: "object" has no attribute "get"  [attr-defined]
- steam/manifest.py:823: error: Value of type "object" is not indexable  [index]
- steam/manifest.py:832: error: Argument 1 to "_get_id" of "PrivateManifestInfo" has incompatible type "object"; expected "Depot"  [arg-type]

core (https://github.com/home-assistant/core)
+ homeassistant/components/arcam_fmj/device_trigger.py:77: error: Unused "type: ignore" comment

paasta (https://github.com/yelp/paasta)
+ paasta_tools/kubernetes_tools.py:1872: error: Incompatible types in assignment (expression has type "Union[Dict[str, Union[str, Dict[str, Any]]], Dict[<nothing>, <nothing>]]", variable has type "Mapping[str, Any]")
+ paasta_tools/kubernetes_tools.py:1872: error: Argument 2 to "get" of "Mapping" has incompatible type "Dict[<nothing>, <nothing>]"; expected "Union[Dict[str, Union[str, Dict[str, Any]]], Mapping[str, Any]]"
+ paasta_tools/kubernetes_tools.py:1893: error: Argument "raw_selectors" to "raw_selectors_to_requirements" has incompatible type "Union[Dict[str, Union[str, Dict[str, Any]]], Dict[<nothing>, <nothing>]]"; expected "Mapping[str, Any]"
+ paasta_tools/kubernetes_tools.py:1893: error: Argument 2 to "get" of "Mapping" has incompatible type "Dict[<nothing>, <nothing>]"; expected "Union[Dict[str, Union[str, Dict[str, Any]]], Mapping[str, Any]]"

github-actions[bot] avatar Jul 25 '22 20:07 github-actions[bot]

I would prefer not merging this. This specific find-replace also makes me uneasy; there are definitely places where Callable[..., object] -> Callable[..., Any] can result in false negatives. Moreover, while typeshed is often happy to make small changes to accommodate specific type checkers, a blanket change like this isn't great and is an unhappy precedent (I would still be okay with just changing the unittest functions or changing a few other locations in response to specific user complaints).

I spent a little bit of time trying to fix the mypy issue, you can see my attempt and list of other things that didn't work out of the box at https://github.com/python/mypy/pull/13221 All the listed things fixed the issue, but caused other regressions and I didn't spend that much time trying to figure everything out. PR should at least be useful in terms of pointers to the relevant code.

hauntsaninja avatar Jul 25 '22 21:07 hauntsaninja

I made the blanket change because users will inevitably run into the same bug on other APIs leading to more confusion, PRs, and overall more wasted time. Ideally mypy would fix the issue but in the meantime this kind of workaround seems appropriate to give users a better experience. That said, I'm happy to only change the unittest methods for now and leave the rest up to you.

ShaneHarvey avatar Jul 25 '22 21:07 ShaneHarvey

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Jul 25 '22 21:07 github-actions[bot]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Jul 25 '22 21:07 github-actions[bot]

Updated to only change unittest (and the DocTestCase subclass of TestCase). Ready for another look.

ShaneHarvey avatar Jul 25 '22 22:07 ShaneHarvey

Marking this as deferred for now, in case we're able to find a good mypy fix in the next several days :-) (Will definitely make sure there's something in place, in typeshed or mypy, before the next mypy release)

hauntsaninja avatar Jul 25 '22 22:07 hauntsaninja

I am opposed to merging this. If there isn't a fix for mypy until the next release, I suggest that mypy ships a patched version of typeshed.

srittau avatar Jul 26 '22 08:07 srittau

there are definitely places where Callable[..., object] -> Callable[..., Any] can result in false negatives.

This may have been true before but I don't think it is anymore now that this PR only changes unittest. My understanding is that there's no typing difference between using Callable[..., Any] vs Callable[..., object] in these stubs (besides working around the mypy bug).

Could we merge this PR or just close it if it will never be accepted?

ShaneHarvey avatar Sep 15 '22 21:09 ShaneHarvey

I am fine with merging this, since none of us has yet managed to find a mypy fix, and given that this now only changes the unittest functions.

AlexWaygood avatar Sep 15 '22 21:09 AlexWaygood

What makes unittest special here?

srittau avatar Sep 16 '22 10:09 srittau

What makes unittest special here?

Well, the ones in unittest have caused a known problem for somebody. That hasn't been shown for any of the other stdlib functions.

AlexWaygood avatar Sep 16 '22 10:09 AlexWaygood

I'm still opposed to this. At this point I don't think we should regress our stubs due to type checker bugs that can be worked around with "# type: ignore". (Unless it's something really fundamental.)

srittau avatar Sep 16 '22 10:09 srittau

I'm still opposed to this. At this point I don't think we should regress our stubs due to type checker bugs that can be worked around with "# type: ignore". (Unless it's something really fundamental.)

I completely understand this perspective!

AlexWaygood avatar Sep 16 '22 10:09 AlexWaygood

I would be more open to this if we had some process that would ensure that we restore the "correct" stubs once the bug is fixed in mypy. And by process I don't mean "someone needs to remember to do this". :)

srittau avatar Sep 16 '22 11:09 srittau

regress our stubs

Can you explain how this would be a regression? IIUC Any and object are functionally identical here.

ShaneHarvey avatar Sep 16 '22 18:09 ShaneHarvey

It's not a functional regression in this particular case, but stylistic one. We invested effort in streamlining callable annotations to use object return types, instead of Any. Changing it back to use Any means potentially more work in the future when investigating why we don't use object instead of Any.

srittau avatar Sep 16 '22 18:09 srittau

regress our stubs

Can you explain how this would be a regression? IIUC Any and object are functionally identical here.

Reintroducing Any instead of object would be a functional regression for mypy users who use the --disallow-any-expr option. But, since this is a fairly obscure and (I believe) little-used mypy option, I agree that this would be a relatively minor regression, which is why I'd be inclined to say "practicality beats purity" for this specific case.

But, I can see it both ways, and agree that the "fault" ultimately lies with mypy here.

AlexWaygood avatar Sep 17 '22 12:09 AlexWaygood

I don't think we'll be merging this — @srittau opposes it, and I don't think any other maintainers are going to come forward to argue in favour of it. Sorry @ShaneHarvey — thanks for the PR anyway.

AlexWaygood avatar Sep 19 '22 22:09 AlexWaygood

No worries. Thanks for the quick responses!

ShaneHarvey avatar Sep 19 '22 22:09 ShaneHarvey

One idea I have is to merge this and just create the reverting PR already, marking it as "deferred". This would mean that there's less chance we forget if mypy fixes the bug. @AlexWaygood Do you think this would be a good idea?

srittau avatar Sep 20 '22 13:09 srittau

One idea I have is to merge this and just create the reverting PR already, marking it as "deferred". This would mean that there's less chance we forget if mypy fixes the bug. @AlexWaygood Do you think this would be a good idea?

I like that idea! We tend to go through open PRs on a fairly regular basis, so I think it would serve as an effective reminder.

AlexWaygood avatar Sep 20 '22 14:09 AlexWaygood

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

github-actions[bot] avatar Sep 20 '22 19:09 github-actions[bot]

Merged! PR to revert is here: #8779.

Thanks for your patience @ShaneHarvey :)

AlexWaygood avatar Sep 21 '22 12:09 AlexWaygood