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

Conversion of Scripts to Python 3 #7

Closed
wants to merge 9 commits into from

Conversation

HelgeCPH
Copy link

Hej Adam,

After a long time I was again reading your book Your Code as a Crime Scene. Thank you for it. It is really well written and inspiring.

Since I wanted to run some of your scripts and since Python 2 is close to EOL, I just thought you might be interested in my changes for Python 3. While doing that I fixed two minor issues
4a552cc
and 359a3b5.

Unfortunately, I do not know how to send a PR for a separate branch, as there are likely people who want to run the Python 2 scripts from the master branch. Please let me know if I should do anything different.

Best,
Helge

@adamtornhill
Copy link
Owner

Thanks a lot @HelgeCPH for your kind words, and thanks for the pull request!

I think we should provide two versions of the scripts: one for Python 2, and one version with your updated Python 3 scripts. We could either introduce a separate folder, e.g. python3 that contains updated versions of all scripts. Or we could maintain a separate fork where we update the documentation to highlight that this is a Python 3 compatible version. We could then link from maat-scripts to the fork.

Which one would you prefer? I'm lending towards the fork.

@HelgeCPH
Copy link
Author

HelgeCPH commented Mar 31, 2019

Thank you for your reply and sorry for my late reply.

I think we should provide two versions of the scripts: one for Python 2, and one version with your updated Python 3 scripts.

Yes, I agree. Likely many people use the Python 2 scripts as they are.

We could either introduce a separate folder, e.g. python3 that contains updated versions of all scripts. Or we could maintain a separate fork where we update the documentation to highlight that this is a Python 3 compatible version. We could then link from maat-scripts to the fork.

Which one would you prefer? I'm lending towards the fork.

Let's do whatever suits you best. If it is the fork then we just do it as you suggested.

@Ryuno-Ki
Copy link

What's the status with this?

I picked up Software Design X-Rays which also refers to this project.

@wonderbird
Copy link
Contributor

wonderbird commented Dec 27, 2021

Hi @adamtornhill, @HelgeCPH and @Ryuno-Ki,

in https://github.com/wonderbird/maat-scripts I have combined @HelgeCPH's changes with the latest commit of @adamtornhill's master branch and resolved the conflicts shown below.

Next steps

Best,
Stefan

@wonderbird
Copy link
Contributor

P.S.: In addition I have resolved a bug that surfaced with the python3 version of the merge_comp_freqs.py script.

@adamtornhill
Copy link
Owner

@wonderbird I have created the branch: https://github.com/adamtornhill/maat-scripts/tree/python3

We should also include a mention of the Python 3 support in the README.md.

@wonderbird
Copy link
Contributor

Thanks @adamtornhill . I have created PR #11 with the intention to replace this PR.

@adamtornhill
Copy link
Owner

Closing this PR since the work was completed in #11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants