Skip to content
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

Fix paginate Decorator Typing #1127

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

max-muoto
Copy link
Contributor

@max-muoto max-muoto commented Apr 12, 2024

Paginating your endpoint transforms the collection return type into one that returns a dict, however, this isn't reflected in the final type.

Let's take this example:

@api.get("/invalid1", response={200: None})
@paginate
def invalid1(request: HttpRequest) -> list[Any]: 
    return [1, 2, 3]

reveal_type(invalid1) # Type of "invalid1" is "(...) -> Unknown

This also leads to typing errors when passing in a pagination class (with Pyright):

@api.get("/invalid1", response={200: None})
 @paginate(PageNumberPagination) # Untyped function decorator obscures type of function; ignoring decorator
def invalid1(request: HttpRequest) -> list[Any]: 
    return [1, 2, 3]

This PR sets up overloading for paginate so that type can be correctly inferred in both cases. Operating under the assumption that assumption that ninja already makes that paginate_queryset returns dict[str, Any]

For our first example, we now get:

@api.get("/invalid1", response={200: None})
@paginate
def invalid1(request: HttpRequest) -> list[Any]: 
    return [1, 2, 3]

reveal_type(invalid1) # (HttpRequest) -> Dict[str, Any]

Other Changes

  • While unlikely, it's not enforced that you paginate a queryset, technically any collection with len is supported. Therefore, I switched some of the QuerySet types to Sequence to better reflect this.
  • Added overload decorated functions to be excluded from coverage requirements, since they're purely for typing.

pagination_params = kwargs.pop("ninja_pagination")
if paginator.pass_parameter:
kwargs[paginator.pass_parameter] = pagination_params

items = func(request, **kwargs)
items = func(request, *args, **kwargs)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To properly utilize ParamSpec (which lets us preserve the call signature), we need to pass in *args. I'm guessing this wouldn't be problematic, but if will be, I can rethink the approach here.

@max-muoto max-muoto marked this pull request as ready for review April 12, 2024 03:12
@max-muoto max-muoto changed the title Improve typing for pagination Fix paginate Decorator Typing Apr 12, 2024
@vitalik
Copy link
Owner

vitalik commented Apr 30, 2024

Hi @max-muoto

Thank you for contribution
Could you fix conflicts with the latest version ?

@max-muoto
Copy link
Contributor Author

Hi @max-muoto

Thank you for contribution Could you fix conflicts with the latest version ?

I'll have to do a bit of reworking with the support for async pagination, but I'll ping you once this is ready again, I'll move it back to a draft in the mean-time.

@max-muoto max-muoto marked this pull request as draft May 13, 2024 22:34
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.

None yet

2 participants