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

Python requirements and version compatibility is not clear #62

Closed
JLBegin opened this issue May 8, 2020 · 2 comments · Fixed by #63
Closed

Python requirements and version compatibility is not clear #62

JLBegin opened this issue May 8, 2020 · 2 comments · Fixed by #63
Assignees
Labels
before publication Critical to fix before we publish question Further information is requested

Comments

@JLBegin
Copy link
Contributor

JLBegin commented May 8, 2020

RayTracing requires tkinter

If you are not installing Python with Anaconda, it is important to keep tcl/tk support during python install since it is required by matplotlib's backend (tkinter). This is no problem if you follow default installation options.
image

Version compatibility

I tested RayTracing and it currently works on Python 3.6 and newer versions (3.7 & 3.8).

Since Python 3.5 is still an active Python 3 release, I tried to install RayTracing, but it fails to run because of a new type hint syntax for variable annotations (PEP 526) used in imagingpath.py:

thetaUp: float = (stopDiameter / 2.0 - A * y) / B
thetaDown: float = (-stopDiameter / 2.0 - A * y) / B

I tested with the following edit on 3.5 which fixes the issue and works for 3.5+.

thetaUp = (stopDiameter / 2.0 - A * y) / B  # type: float
thetaDown = (-stopDiameter / 2.0 - A * y) / B  # type: float

Conclusion

  • We need to choose if we want to discard PEP 526 to support Python 3.5+ or keep it and 'only' support 3.6+.
  • README.md needs to be updated to tell the required python version.
@JLBegin JLBegin added the question Further information is requested label May 8, 2020
@JLBegin
Copy link
Contributor Author

JLBegin commented May 8, 2020

I guess the tcl/tk requirement is a no-brainer, but if someone gets an ImportError: No module named tkinter then this is how you fix it.

@JLBegin JLBegin self-assigned this May 8, 2020
@dccote
Copy link
Contributor

dccote commented May 10, 2020

I really want type hints. I suggest we recommend 3.6 and above, unless there are very good reasons to limit to 3.5.
Type hints are very important for educational purposes and clarity. It is one of the design goal, so we should not compromise on this considering it is only a minor version 3.5->3.6 requirement. I think we need to modify setup.py to reflect that.

@dccote dccote added the before publication Critical to fix before we publish label May 10, 2020
@JLBegin JLBegin linked a pull request May 10, 2020 that will close this issue
@dccote dccote closed this as completed May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
before publication Critical to fix before we publish question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants