Add type hints to the toolbar and middleware modules #2227
Conversation
|
@tim-schilling @matthiask I suggest we start using https://github.com/typeddjango/django-stubs for the typing |
debug_toolbar/toolbar.py
Outdated
| return getattr(self.request, "csp_nonce", None) | ||
|
|
||
| def get_panel_by_id(self, panel_id): | ||
| def get_panel_by_id(self, panel_id: str) -> "Panel": |
There was a problem hiding this comment.
Sorry, I'm likely missing something basic. Why can't we use -> Panel since we have the TYPE_CHECKING import check?
|
@tim-schilling have you had time to look at the new changes? |
tim-schilling
left a comment
tim-schilling
left a comment
There was a problem hiding this comment.
This is coming along nicely, thank you for continuing to work on it. I think there are a few arguments in the method signatures that are missing. I think there may be too many TYPE_CHECKING protections as well, so we should be able to simplify things some more.
Not to be solved right now:
I'm also realizing we need a way to evaluate this programmatically. It's going to be near impossible to keep this inline without some type checking.
debug_toolbar/toolbar.py
Outdated
|
|
||
| @classmethod | ||
| def from_store(cls, request_id, panel_id=None): | ||
| def from_store(cls, request_id, panel_id=None) -> "StoredDebugToolbar": |
There was a problem hiding this comment.
Should the rest of this signature (and debug_toolbar_urls) have types? Or how are you cutting things off?
There was a problem hiding this comment.
I think they should but i was trying to do things incrementally
debug_toolbar/toolbar.py
Outdated
| class StoredDebugToolbar(DebugToolbar): | ||
| def __init__(self, request, get_response, request_id=None): | ||
| def __init__( | ||
| self, request: HttpRequest, get_response: "GetResponse", request_id=None |
There was a problem hiding this comment.
| self, request: HttpRequest, get_response: "GetResponse", request_id=None | |
| self, request: HttpRequest, get_response: GetResponse, request_id=None |
I think this is also missing the type definition for request_id
There was a problem hiding this comment.
I was not really sure what to use for the type of request id that why i didnt type it
tim-schilling
changed the title
tim-schilling
changed the title
tim-schilling
left a comment
tim-schilling
left a comment
There was a problem hiding this comment.
Alright, I think this is good to go now. There's a bug / bad design with the stored toolbar class, but I think we can punt on it for now.
3 participants
Description
This PR adds type hints to the middleware and toolbar modules. It's based on the work of @leandrodesouzadev in this pr #1848
Fixes #1705
Checklist:
docs/changes.rst.