Support positional args in github.paginate

[brandonchinn178]

Broken out from #194

I know I can pass as kwargs, but the signature has *args: P.args, so I would expect positional arguments to work too. If they're not intended to work, it should either be removed or documented how to use them.

[yanyongyu]

I think this feature could be: more flexible pagination.

After taking a look at the latest docs, GitHub can now uses the link header to do pagination. Octokit also supports this now (ref).

Currently, the recommended pagination method is to use the link header and the page/per-page form can be the alternative method.

Since the link method does not need to handle page num manually. The page, per-page can be directly removed and passed as kwargs. note that this is still a breaking change.

[yanyongyu]

After doing some tests on this, i find that we still can not pass the positional arguments directly after the request method. Because of the map_func, we still need to do some like this:

for issue in g.paginate(g.rest.issues.list_for_repo, None, "owner", "repo"):
    ...

The map_func cannot be moved after the positional arguments because this signature:

def paginate(
    request: R[CP, list[RTS]],
    *args: CP.args,
    map_func: None = None,
    **kwargs: CP.kwargs,
):
    ...

is invalid.

We cannot concanate keyword arguments as it is rejected in PEP612. So, it still need to be positional arguments before the request's args.

[brandonchinn178]

Ah yes, I found that issue in another part of our code.

Fundamentally, I don't think *args is useful at all after map_func, since, as a user, I would never want to write github.paginate(f, None, arg1, arg2). Unfortunately, it seems like Python doesn't support only doing P.kwargs without P.args.

Might be worth explicitly recommending keyword args with paginate. Alternatively, perhaps recommend using functools.partial instead?

github.paginate(
    functools.partial(
        github.rest.orgs.list_webhook_deliveries,
        hook_org,
        hook_id,
    )
)

[yanyongyu]

if use functools.partial to wrap the request method, it will be hard to detect sync/async and perform validation. Maybe using keyword args is the best way.

[brandonchinn178]

It seems like you're using inspect.iscoroutinefunction, right? Looks like it should work with partial?

https://stackoverflow.com/questions/67020609/partial-of-a-class-coroutine-isnt-a-coroutine-why

[yanyongyu]

functools._unwrap_partial is not a public api for extracting the original func from partial. So, I don't think handling partial specially is a good approach.

[brandonchinn178]

You wouldn't have to use functools._umwrap_partial, you could just implement it yourself:

while isinstance(o, functools.partial):
    o = o.func
return inspect.iscoroutinefunction(o)

[brandonchinn178]

Thank you!!