Add pyright and cleanup the codebase #9
Conversation
This also fixes a slight issue where if a list/tuple was passed into the registry class as data, it was treated as the reversed data rather, and reversed data was the actual data.
For some reason, the from_list method was originally making the instance in which it then (optionally) checked for duplicate packets based on ID. However during the original parsing, packets were stored as dicts with IDs as keys, this means even if there were some duplicates, they'd just get overridden and we'd only pass the last packet in the list with that ID. This meant the duplicate check would never actually catch any duplicate packets. This logic was also not ideal because it went through the list of packets server times for no reason, when it could all be done in a single loop. This is way more efficient which would become noticable if the list of packets was long enough. It also fixes an invalid type hint, which specified the packets argument should be a List[Type[Packet]], however this would mean we expect the list to contain the actual "Packet" class (or it's subtypes), when in fact we want instances of this type. Not only that, but the function can't even process any other type of packets than ClientBound and ServerBound ones, so we shouldn't specifiy that it takes any arbitrary packet, but rather a union of the two.
In 7dcbfa9, I wrongly assumed that the function was taking instances, when in fact it was taking classes, this reverts that part of the commit.
Originally, this return type was "Packet, which I've then change to a Union of 2 subtypes of Packet, however this type was actually supposed to be a Type[Packet], so this changes the return type to the type (class) of the union of these 2 subtypes
|
I mean pyright requires node js etc, and that python package would install it as well Have the python native options, like mypy (iirc) been considered? |
i mean, npm will only be installed in the python's virtual environment (assuming you did use the python pyright package for installation with poetry), not on the machine's global environment, so you shouldn't have any issues with it colliding with your system versions, or with it cluttering your system or anything like that.
I do know about other alternatives, written directly in python, such as mypy or even less common things like pytype or pyre. However most of these alternatives are a lot slower than pyright, and have way worse support when it comes to newer typing features (such as @the456gamer |
Turns out it's actually very hard, and perhaps impossible to properly type-hint the Registry class because python's typing doesn't have support for Higher Kinded TypeVars. The class is also very complex (even if it doesn't have that much code) and allows for way too many edge cases, which would probably result in a bunch of overloads and type ignores, making the code even more convoluted than it already is. Instead, this removes all of the generic behavior to the Registry class and improves some minor code quality issues. This isn't ideal, since it means it'd going to rely on typing.Any, but due to this difficulty and complexity, it's justified.
Accepting both a mapping and a sequence from init as data is weird, it just makes the code more messy and it's not a necessary thing that needs to be supported by the main constructor, it's an alternative way to pass data over, which means it should use an alternative constructor. This adds `from_sequence` classmethod constructor to address this.
Since pyright 1.1.224, strictParameterNoneValue is now true by default, causing typing errors on things like `x: int = None` which instead requires explicit: `x: Optional[int] = None` to pass now.
This adds pyright type checker to enforce proper type-hinting and type propagation everywhere. With it, the
StrictABC's type-enforcement functionalities are no longer necessary, and so they will be removed.I've also done some general codeabse cleanup for non-ideal things I found in the code while making the project pyright compliant and fixing several type-hints. These cleanup changes sometimes even fix major bugs, which went previously unnoticed but were catched by pyright. Note that these bugs would never make it into production if the test coverage was better.
Tasks:
CONTRIBUTING.md