📝 Rename Websocket by WebSocket#272
📝 Rename Websocket by WebSocket#272andrewgodwin merged 6 commits intodjango:mainfrom Kludex:fix/websocket-type-name
Conversation
This is fundamentally unnecessary, so the rejecting this PR is understandable. But the correct name is WebSocket.
|
I don't object to it, but if you want to land this you'll need to do it in a backwards-compatible fashion (so the old names work but raise a warning) |
|
I've used PEP 562 idea here, but as it was introduced in 3.7, so I added the backport implementation for 3.6 as requirement. I can also copy the file if you wish so, and we don't add it as dependency. I've also added tests to check that the warning is being raised on the wanted types. |
|
I'd rather just do it the "old fashioned" way if we could - expose them as actual top-level module objects rather than bringing in extra code just for this. |
|
I'm not following you. 🤔 How am I supposed to warn the user about the deprecation at import time without this implementation? Or you just meant to not use |
|
Oh, I forgot they were type classes for a second, sorry. Thought they were actual classes where you could do it with a mixin on instantiation. In that case, let's vendor pep562 in provided the license matches - I don't want more dependencies! |
| self._get_dir: Optional[Callable[..., List[str]]] = getattr( | ||
| self._module, "__dir__", None | ||
| ) | ||
| sys.modules[name] = self # type: ignore[assignment] |
There was a problem hiding this comment.
sys.modules expects a Module and self is a Pep562, jfyk.
|
There's a mismatch between the formatters, EDIT: If I apply either configuration, I'll have a lot more diffs here... Should I open another PR to fix this mismatch, and which one do you prefer, 88 or 119? EDIT2: The problem is with this big import: from typing import (
Any,
Awaitable,
Callable,
Dict,
Iterable,
List,
Optional,
Tuple,
Type,
Union,
)Which |
|
Notes about the changes:
|
|
I think sticking with |
|
Thank you! 🎉 |
This is fundamentally unnecessary, so the rejecting this PR is understandable. But the correct name is WebSocket.