Skip to content

User/badgerj/model fixes#168

Open
badgerj wants to merge 2 commits into
ToyKeeper:trunkfrom
badgerj:user/badgerj/model-fixes
Open

User/badgerj/model fixes#168
badgerj wants to merge 2 commits into
ToyKeeper:trunkfrom
badgerj:user/badgerj/model-fixes

Conversation

@badgerj

@badgerj badgerj commented Jun 2, 2026

Copy link
Copy Markdown

This fixes up a few files to make them python3 compliant.
This would work well with the dockerfiles to keep things compliant.

@badgerj

badgerj commented Jun 2, 2026

Copy link
Copy Markdown
Author

@ToyKeeper / @SammysHP I rebased this to trunk just now.

It would be great if I could get your eyes on this.
The concept would be to also have an idempotent test environment in Docker.

@ToyKeeper

Copy link
Copy Markdown
Owner

This script is already python3 compliant, no? The shebang could say "python3" instead of "python", but many systems already use python3 by default... and the way it is written now, it works regardless of whether the host has "python" mapped to python2 or python3.

Looking at the changes, it does not appear to be about python3 support, but rather, renaming variables and conforming to a lint tool that I don't use. I am aware of Black, and made a decision ages ago not to use it because I don't like the style it enforces. Some of the changes seem okay, like I had an unused import, and used a weird method of stripping a path component once, and I could probably stand to use the new list.sort(key) API instead of Schwartzian transforms... but otherwise the patch consists of things I'd want to undo before merging. Like, I use single-quoted strings on purpose.

It's kind of moot though, because this script was just a quick one-off to generate the MODELS file years ago, and that file has been edited manually ever since, due to the need for extra legacy entries and notes. So the script isn't really used any more. Perhaps I could add more features to handle legacy models and notes, but ... it's easier to just edit the MODELS file directly.

@badgerj

badgerj commented Jun 15, 2026

Copy link
Copy Markdown
Author

Sure. My idea was to just clean it up, and make sure it is PEP8/Flake8 compliant.

Along with the Dockerfile change I was proposing, this would start to introduce code format compliance so that all PRs conform to @ToyKeeper 's and the general C/C++/Python modern coding practices along with consistent formating.

If the script is entirely no longer needed, then I can change this PR to remove the file.

I can easily change it to do that if you like.

I was just trying to slowly introduce myself into the code base, demonstrate that I'm here to make things better and easier for others to contribute and reduce the load on the people doing reviews.

Let me know and I can change this to a removal.

My rationale is I do not like "editing things directly" because it leads to errors. If you write infrastructure as code, it will come out the same every time.

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.

2 participants