Skip to content
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

Migrate project to Python3 #8

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

ImanolBarba
Copy link

In most modern operating systems nowadays python2 is deprecated and not even shipped anymore (most linux distros, macOS too). This means no one can run this code unless they somehow get an unsupported copy of python2 and deal with the mess of adding the necessary dependencies.

This PR migrates the whole project to Python3, and now anyone can run it with contemporary python versions.

I've tested every single command on CLI and GUI.

I'm aware there's a mymcplus project that did this migrating and added a couple features and tests, but that has a GPL3 license and this is Public Domain. I'd like the Public Domain version to at least work so it can still be used.

I've done my best to split the changes in groups so it's easier to review, but it's still a large PR, sorry about that.

Making exceptions compatible with Python3 syntax
Replacing python2 style print calls with python3 ones
Apparently there was mixed spaces/tabs in these lines
Python3 uses str as the type, not string.
Python3 uses StringIO from io package directly
When decoding base64, python3 works with bytes, so we need in fact BytesIO, instead of StringIO
Python3 automatically converts these divisions to floats, which causes incorrect typing issues as well as rounding/off-by-one issues. These need to be explicitly converted to integer divisions
Python3 has different handling of string encoding. Additionally, all functions dealing with data don't return strings, but bytearrays, so there were a lot of typing errors that needed to be fixed
In python3, `range` is equivalent to `xrange`, which is no longer valid
Python3 requires the `next` function to be called `__next__`, which is implicitly called when generating the next value using the iterator
Everything else that was wrong or was making the linter unhappy
@@ -1148,7 +1148,7 @@ def search_directory(self, dir, name):
start = dir.tell() - 1
if start == -1:
start = 0
for i in range(start, len(dir)) + range(0, start):
for i in list(range(start, len(dir))) + list(range(0, start)):
Copy link
Author

Choose a reason for hiding this comment

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

This should've been in the "Random fixes" commit, but still kinda relevant

@ImanolBarba
Copy link
Author

I added a couple more commits to make the code quality analysis pass

@ImanolBarba
Copy link
Author

Messed up the push, need to redo it

@ImanolBarba ImanolBarba force-pushed the python3 branch 2 times, most recently from ae87464 to 3adf9aa Compare July 9, 2023 20:10
@ImanolBarba
Copy link
Author

Finally rebased properly, sorry for the churn

@ImanolBarba
Copy link
Author

nvm, autoformatter butchered something, I'll send the changes later after making sure it works

Less compatibility issues this way
@motorto
Copy link

motorto commented Sep 26, 2023

whats the status of this: tried it in linux but still getting a bunch of errors.

./mymc.py: line 27: from: command not found
~``

@ImanolBarba
Copy link
Author

whats the status of this: tried it in linux but still getting a bunch of errors.

./mymc.py: line 27: from: command not found
~``

The original script did not have the usual shebang sequence to specify the interpreter, so your machine is just trying to use the current shell (likely bash or similar).

Same as the original, you need to run it like:

python3 mymc.py

The alternative is to add:

#!/usr/bin/env python3

At the beginning of the script. I'm happy adding the header though if you'd like

@motorto
Copy link

motorto commented Oct 15, 2023

In case you are not aware: https://git.sr.ht/~thestr4ng3r/mymcplus

@ImanolBarba
Copy link
Author

In case you are not aware: https://git.sr.ht/~thestr4ng3r/mymcplus

Yeah I did mention mymcplus in the commit (I think) .

I just want the public domain licensed version to work as well

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