archinstall icon indicating copy to clipboard operation
archinstall copied to clipboard

Fix mypy complaints in models.dataclasses.py

Open demipaki opened this issue 3 years ago • 4 comments

🚨 PR Guidelines:

New features (v2.2.0)

All future work towards v2.2.0 is done against master now.
Any patch work to existing versions will have to create a new branch against the tagged versions.

Describe your PR

I fixed all of the complaints from mypy in models.dataclasses.py. This was in attempt to help the library be more compliant with PEP 561 as discussed in issue #114.

You'll notice that in the comparison dunder functions, I replaced the type from VersionDef to object. This is because these methods can expect any type of object, which is why I raise NotImplemeted if it gets compared with any other type than itself. Inspired by this: https://stackoverflow.com/questions/54801832/mypy-eq-incompatible-with-supertype-object

Testing

In another commit I wrote a few unit tests for the VersionDef class. That being said I noticed that there isn't a preexisting test suite yet (that's in another branch, presumably coming soon). So feel free to disregard that commit if you want.

demipaki avatar May 15 '22 17:05 demipaki

I'm not sure why, but this PR breaks packages.py.. They shouldn't have any correlation but yet it breaks? screenshot

Torxed avatar May 16 '22 07:05 Torxed

@demipaki If this file is fully mypy compliant now, could you add it to the github action hook as well https://github.com/svartkanin/archinstall/blob/66765e1735e950a6d40c6d44c5615b065dc1d309/.github/workflows/mypy.yaml#L18-L18? This way it will remain clean since nobody can make modifications anymore that are not compliant :)

svartkanin avatar May 16 '22 11:05 svartkanin

I've just pushed commits to this branch by accident without doing the test, that broke packages.py, that @Torxed mentioned...

I'll request another review once I've done the test myself. Sorry, this is my first time contributing to a project on github :sweat_smile:

demipaki avatar May 16 '22 18:05 demipaki

No worries hehe, and critique sounds so harsh :P More a gentle nudge ^^ I appreciate the PR!

Torxed avatar May 16 '22 19:05 Torxed