Skip to content

Migrate from Python2 to Python3 #681

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

Merged
merged 9 commits into from
Jul 27, 2022
Merged

Conversation

justice-adams-apple
Copy link
Collaborator

Pull Request Description

Updated all Python scrips in the repository to support Python3 and dropped support for Python2. This included

  • Fixing syntax errors throughout
  • Using pythonic context managers to handle file management
  • Using a proper Enum class for results handling
  • Cleaning imports and updating shebang lines

I also took it upon myself to rename project_future.py -> project.py as we originally intended

Acceptance Criteria

Not applicable here

@justice-adams-apple
Copy link
Collaborator Author

Labelled as WIP for now because I need to do some manual testing quick while the CI build kicks off.

@justice-adams-apple
Copy link
Collaborator Author

@swift-ci test

Copy link
Member

@shahmishal shahmishal left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@justice-adams-apple justice-adams-apple changed the title [WIP] Migrate from Python2 to Python3 Migrate from Python2 to Python3 Jul 25, 2022
@justice-adams-apple
Copy link
Collaborator Author

@shahmishal Will merge this today, just one question:

I saw some performance degradation when using python 3.7 to test my changes, in particular, things were just taking longer. After profiling the code I saw that the subprocess module was slower. This was fixed for macOS in 3.8+
https://docs.python.org/3.8/whatsnew/3.8.html#optimizations

And after updating to 3.9, I no longer saw an issue. The question is: Should we note on the readme that this repository is now targeting python 3.8+, just to be explicit for end-users? If so, I can update this branch quickly to include a change for the readme

@justice-adams-apple
Copy link
Collaborator Author

@swift-ci test

@shahmishal
Copy link
Member

@shahmishal Will merge this today, just one question:

I saw some performance degradation when using python 3.7 to test my changes, in particular, things were just taking longer. After profiling the code I saw that the subprocess module was slower. This was fixed for macOS in 3.8+ https://docs.python.org/3.8/whatsnew/3.8.html#optimizations

And after updating to 3.9, I no longer saw an issue. The question is: Should we note on the readme that this repository is now targeting python 3.8+, just to be explicit for end-users? If so, I can update this branch quickly to include a change for the readme

Let's update the README to contain the info, it's ok to do this in new PR.

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