-
-
Notifications
You must be signed in to change notification settings - Fork 8
Fix reported mypy issues #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
jorio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR! Please see a few comments in my review.
Annotating the serialization/deserialization code is a bit tricky.
|
|
||
| class JSONEncoderBase16Fallback(json.JSONEncoder): | ||
| def default(self, o: Any): | ||
| __slots__ = () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think __slots__ are worth the maintenance overhead in a small project like this. Does mypy need them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but mypy will give you warnings when accessing attributes that don't exist in slots, and Python will use memory more efficiently when objects have __slots__ defined
Co-authored-by: Iliyas Jorio <iliyas@jor.io>
Co-authored-by: Iliyas Jorio <iliyas@jor.io>#
|
Thank you for these fixes! There seems to be an issue somewhere in the PR that prevents PICT resources from being converted. For example, Nanosaur 1's Application.rsrc has 3 PICT resources (128, 129, 130). On the master branch, (Sorry for the inconvenience – I really ought to write a proper test suite for this tool.) |
In this pull request, we fix type issues as reported by mypy.