Skip to content

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.

@alanjds
Copy link
Owner

alanjds commented Mar 13, 2025

I will close this here because looks like it got fixed on typeddjango/djangorestframework-stubs#610 already.

Please reopen if I am mistaken.

@alanjds alanjds closed this Mar 13, 2025
@intgr
Copy link
Contributor Author

intgr commented Mar 13, 2025

The changes in djangorestframework-stubs don't really solve the issue I described in this PR. But I also understand not wanting to duplicate the signatures here downstream, fair enough.

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