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

PoC: Add detailed hints for Nested*Router.__init__() #345

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

Conversation

intgr
Copy link
Contributor

@intgr intgr commented May 10, 2024

I'm not sure if you want this complexity, but I wanted to demonstrate what can be still improved.

Because NestedMixin is declared as:

    def __init__(
        self,
        parent_router: SimpleRouter | DefaultRouter | NestedMixin,
        parent_prefix: str,
        *args: Any,
        **kwargs: Any
    ) -> None:

Typecheckers cannot see beyond the *args: Any, **kwargs: Any arguments, they will allow literally anything as long as the first 2 arguments are satisfied.

In order to fix this, it would be necessary to add complete signatures, duplicating the arguments from DRF itself.

@alanjds
Copy link
Owner

alanjds commented May 10, 2024

Typecheckers cannot see beyond the *args: Any, **kwargs: Any arguments, they will allow literally anything as long as the first 2 arguments are satisfied.

In order to fix this, it would be necessary to add complete signatures, duplicating the arguments from DRF itself.

I see.

What I would like is to inform what this mixing "soft-inherits" from, but without really creating an inheritance. Duplicating code from DRF is not something that I want. In fact, DRF made some small internal adjustment so drf-nested-routers got able to override just a little part of the routers, instead of copy-pasting some lines.

That makes me think that DRF may be open to small changes that facilitate what you want, if this does not reduce their code readability.

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.

2 participants