Skip to content

Clean up python file handling - #1268

Merged
ethteck merged 4 commits into
pmret:mainfrom
Darxoon:main
Sep 26, 2026
Merged

ethteck merged 4 commits into
pmret:mainfrom
Darxoon:main

Conversation

@Darxoon

@Darxoon Darxoon commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

While working on windows support for dx, i found that lots of python code doesn't explicitly specify an encoding when calling open, which causes an issue on windows where it defaults to cp1252 for some reason. This also made me find some instances of file.close() being called explicitly and lots of instances of "r" being omitted (which can be confusing because it's not immediately clear what python's default behavior is in this case).

While the encoding changes aren't technically necessary here as the decomp doesn't support non-WSL windows, it is still good to have.

@scarletcafe scarletcafe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ironically PEP 686 means this behaviour will be default as of a month from now anyway but it's always good to be explicit.

@Darxoon

Darxoon commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

oh that's good to hear because the current behavior is stupid, also really funny timing, but of course even then it'd be good to support python versions before 3.15

@ethteck

ethteck commented Sep 9, 2026

Copy link
Copy Markdown
Member

Thanks for doing this! It looks like there's one file that needs a formatting fix according to black - do you mind doing that?

@BowserSlug

Copy link
Copy Markdown
Collaborator

🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀🚀 This PR matches 71 functions (+0.77%) on jp.

@Darxoon

Darxoon commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

How do I fix this?

@scarletcafe scarletcafe left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on what the closest env I can get to what it's using reports, it's just complaining about the single quote usages here.

The GHA should probably be changed to use black --diff instead of black --check, and the Python version should be bumped up from 3.8 since it's EOL.

Comment thread tools/asm_sizes.py Outdated
Comment thread tools/disasm_hud_element_animation.py Outdated
Comment thread tools/disasm_script.py Outdated
Comment thread tools/disasm_script.py Outdated
Co-authored-by: Devon R <8049998+scarletcafe@users.noreply.github.com>
@Darxoon

Darxoon commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks

@Darxoon

Darxoon commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Can this be merged?

@ethteck
ethteck merged commit 1104f1f into pmret:main Sep 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants