-
Notifications
You must be signed in to change notification settings - Fork 37
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
fix: compatible with sympy 1.6(#1023) #1028
Conversation
@@ -205,7 +206,8 @@ def octoutput(x, et): | |||
f.text = d2hex(x.real) | |||
f = ET.SubElement(a, "f") | |||
f.text = d2hex(x.imag) | |||
elif isinstance(x, sp.compatibility.string_types): | |||
elif isinstance(x, str) or \ | |||
(sys.version_info[0] <= 2 and isinstance(x, unicode)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of 1.6, SymPy no longer itself supports Python 2, so there should be no need to use Python 2 compatibility idioms in this codebase, unless you are still supporting SymPy 1.5 and Python 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks. I kind of am, b/c my windows package relies on something (py2exe) that doesn't work on Py3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The py2exe on PyPI apparently supports Python 3 https://pypi.org/project/py2exe/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last I checked I was held up by manthey/pyexe#25
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I suggest switching to pyinstaller? It can also produce a single exe binary which packs all the needed dlls. It is also actively developed unlike pyexe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cbm755: The Windows installer for Octave 6.x (built with MXE Octave) bundles a minimal version of python 3.8, sympy 1.4 and the symbolic package 2.9.0. So, it should no longer be necessary to provide a separate Windows installer for the symbolic package. (At least for newer versions of Octave.)
It would probably be possible to change to different versions of python or sympy in MXE Octave if that simplifies an update of the symbolic package.
If this is holding back getting the symbolic package to work with newer versions of sympy, you could probably just ignore backward compatibility with python2 (for Windows).
Thanks! I've been ignoring this project for too long: must clean these up and do a new release. Will look later |
@cbm755 Hi, its later, mind taking a look? |
I think that Python 2 compatibility should not hold this PR, and that supporting an up-to-date version of sympy should be top priority. Python 2 support can be added later. |
Hi, this PR fixes #1023.
However, I'm not quite sure how to edit
inst/private/check_and_convert.m
to support python2. Help needed!