Typing fixes

[bryanforbes]

Summary

• Make some Cog methods positional-only
• Fix maybe_coroutine and MaybeCoroFunc
• Mark stub functions that can be coroutines as returning MaybeCoro

Checklist

• [x] If code changes were made then they have been tested.
  • [x] I have updated the documentation to reflect the changes.
• [ ] This PR fixes an issue.
• [ ] This PR adds something new (e.g. new method or parameters).
• [ ] This PR is a breaking change (e.g. methods or parameters removed/renamed)
• [ ] This PR is not a code change (e.g. documentation, README, ...)

[Rapptz]

Make some Cog methods positional-only

Isn't this outside of the scope of this PR?

Mark stub functions that can be coroutines as returning MaybeCoro

I don't think I agree with this change. Is there a reason it was done?

[bryanforbes]

Make some Cog methods positional-only

Isn't this outside of the scope of this PR?

I consider positional-only changes to be typing changes. If you'd like, I can split this out into another PR.

Mark stub functions that can be coroutines as returning MaybeCoro

I don't think I agree with this change. Is there a reason it was done?

The methods I've changed to return MaybeCoro[...] are technically defining a protocol without being an explicit Protocol. Because the implementation methods can return either a coroutine or the sync value, the protocol should indicate it so that users using strict mode don't get an incompatible override error. Consider the following:

class ThingGetter:
    def get_thing(self) -> int | string:
        raise NotImplementedEror

class A(ThingGetter):
    def get_thing(self) -> int:  # no incompatible override error
        return 1

class B(ThingGetter):
    def get_thing(self) -> str:  # no incompatible override error
        return 'thing'

def get_thing_from_getter(getter: ThingGetter) -> int | str:
    return getter.get_thing()

get_thing_from_getter(A())  # correct
get_thing_from_getter(B())  # also correct

Now consider the MaybeCoro[...] case:

class ThingGetter:
    def get_thing(self) -> int | Coroutine[Any, Any, int]:
        raise NotImplementedEror

class A(ThingGetter):
    async def get_thing(self) -> int:  # no incompatible override error
        return 1

class B(ThingGetter):
    def get_thing(self) -> int:  # no incompatible override error
        return 2

async def get_thing_from_getter(getter: ThingGetter) -> int:
    return await maybe_coroutine(getter.get_thing)

async def main() -> None:
    await get_thing_from_getter(A())  # correct
    await get_thing_from_getter(B())  # also correct

[Rapptz]

The set of users who use strict is incredibly small and generally aren't the type of people I'm trying to please in the library. The incompatible method error for example will always trigger when a generic is passed as bound even though it's technically okay.

A big issue I have with this is usability. A lot of tooling helpfully autocomplete methods when overriding and this one will give users a bizarre signature to override that includes a private alias type.

[bryanforbes]

A big issue I have with this is usability. A lot of tooling helpfully autocomplete methods when overriding and this one will give users a bizarre signature to override that includes a private alias type.

This is a really good concern to have. Let me know what you'd like me to do with this PR. I can split it into two: one for fixing maybe_coroutine() and one for the positional-only params.

[Rapptz]

I think just the maybe_coroutine fixes are okay. The positional-only parameter change seems like an unnecessary breaking change in this case.

[bryanforbes]

This has stagnated and the maybe_coroutine() changes have been handled in 06c257760bdedd39c37a7eb12f0338ac60b48c20. Closing this.