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

updates to make pre mjv changes work with python 3.9 #8

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

Conversation

obwalton
Copy link
Member

@obwalton obwalton commented Dec 1, 2021

Keep in mind the goal of this PR was to get ImageEchelon working with Python 3 (in this case 3.7, it appears that python 3.9 has breaking changes for the elo package). I wanted something working that would allow me to stand up an instance on a modern vm with new data from the Burgess Lab.

@obwalton obwalton requested review from gbeane and bergsalex December 1, 2021 14:54
Copy link

@gbeane gbeane left a comment

Choose a reason for hiding this comment

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

this looks pretty straightforward to me

Comment on lines 1 to +3
from __future__ import print_function
from __future__ import absolute_import
from __future__ import division
Copy link

Choose a reason for hiding this comment

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

I'm not sure these from future imports should be kept.

I assume these are here because of concerns about maintaining compatibility with Python2, which I'm not sure is necessary now that Python 2 is no longer supported/updated. Also, this file doesn't appear have any prints, division, or absolute_imports, so I don't think these will make a difference for Python 2 vs Python 3 anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, those were all added by the py2 -> py3 tools I used to migrate the code. I can remove them.

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