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 3 support? #1

Closed
bilderbuchi opened this issue May 5, 2015 · 51 comments
Closed

Python 3 support? #1

bilderbuchi opened this issue May 5, 2015 · 51 comments

Comments

@bilderbuchi
Copy link

It seems OmniORB is the holdup keeping Python support to version 2. As I was digging around, I found that apparently the dev version/upcoming version offer Python 3 support, judging from some commit messages since 07/2014 and even some parts of the current version support Py3, so I thought I'd leave that here.

@bilderbuchi
Copy link
Author

I just saw that OMEdit got rid of Corba recently - does that mean that Corba in OM is on its way out, or is this restricted to OMEdit? If the former that would probably help with a Python3 transition.

@sjoelund
Copy link
Member

The Python interface could indeed be changed to use dynamic loading as well. However, it is not as simple as for the C++ interface since that interface can access the macros used to start the garbage collector, create MetaModelica strings, and so on. For Python, all such interactions would probably need to be performed on a separate thread. Which means coding a Python C wrapper instead of CORBA...

@bilderbuchi
Copy link
Author

So, omniORB/omniORBpy 4.2.1 is now out, having full Python3 support (Python 3.2+/2.5+). :-)

@sjoelund
Copy link
Member

4.2.1: This release has the following known limitations: Only Python 2.x is supported.
So their biggest new feature was not removed from the limitations of the release notes :D

@bilderbuchi
Copy link
Author

yeah, I saw this too, and I concluded that it was just an oversight (probably late-night release preparation ^^).

@grantstephens
Copy link
Contributor

Any news on Python3 support? Just starting to run into some other issues that it seems python3 might be able to solve but OMPython is holding me back from updating.

@bilderbuchi
Copy link
Author

not afaik, but OmniORB in fact seems to be py3-compatible, I see py3-related commit messages in the log.

@hkiel
Copy link
Member

hkiel commented Jul 14, 2016

From the build for windows it seems Python3 really is supported:
https://sourceforge.net/projects/omniorb/files/omniORBpy/omniORBpy-4.2.1/omniORBpy-4.2.1-win64-python35.zip

@hkiel
Copy link
Member

hkiel commented Jul 15, 2016

The most annoying thing I encounter with Python3 is that it clashes with Python2 installation.
I have everything defaulted to py2, but when I start jupyter-notebook (from 2.7 version) I get lots of error messages from 3.5.

Seems to have to do with my sudo python3 setup.py install. When I revert to sudo python2 setup.py install it works again (seems to install in the wrong place?)

@bilderbuchi
Copy link
Author

bilderbuchi commented Jul 15, 2016

I never had those problems on my Linux box, but I tended to install packages with pip - you only have to take care to use pip and pip3 as appropriate (and never with sudo), then the packages stay nicely separated in your home directory. However, I only ever used ipython notebook with py3.

Nowadays, I'm using Anaconda for both Windows and Linux, and don't encounter many problems.

@thorade
Copy link
Contributor

thorade commented Jan 4, 2017

You can also use python -m pip install SomePackage to make sure the correct/corresponding pip is used.
https://docs.python.org/3/installing/
https://docs.python.org/3/installing/#work-with-multiple-versions-of-python-installed-in-parallel

@dietmarw
Copy link
Contributor

dietmarw commented Jun 7, 2017

I think at least for Windows the Python 3 issue is solved since omniorb > v4.2.0 supports Python 3. I have not tested this in practice. On Linux it is a different matter since the omniorb packages are currently out of date.

@dietmarw
Copy link
Contributor

dietmarw commented Jun 7, 2017

So I did some tests and now found that OpenModelica is actually packaging OmniOrb (at least in the Windows installer). So if the OpenModelica team would be so kind to update the libraries in \lib\python then we might have a working OMPython installation prepared for Python 3

@adeas31
Copy link
Member

adeas31 commented Jun 15, 2017

ZeroMQ is added as an alternative to omniORB in #23. However when I tried OMPython with Python 3.6 then I got errors from pyparsing library.

@bilderbuchi
Copy link
Author

What errors? Omniorb being out of the way is a big step towards py3, i think, great work!

@adeas31
Copy link
Member

adeas31 commented Jun 15, 2017

Python 3.6.1 (v3.6.1:69c0db5, Mar 21 2017, 18:41:36) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from OMPython import OMCSessionZMQ
>>> omc = OMCSessionZMQ()
2017-06-15 21:49:52,880 - OMPython - INFO - OMC Server is up and running at file:///C:/Users/adeas31/AppData/Local/Temp/openmodelica.port.d511f1be28354de8adf580a2b0f7c416
>>> omc.sendExpression('getVersion()')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python36\lib\site-packages\ompython-2.0.7-py3.6.egg\OMPython\__init__.py", line 554, in sendExpression
    answer = OMTypedParser.parseString(result)
  File "C:\Python36\lib\site-packages\ompython-2.0.7-py3.6.egg\OMPython\OMTypedParser.py", line 82, in parseString
    result = omcGrammar.parseString(string)
  File "C:\Python36\lib\site-packages\pyparsing-2.2.0-py3.6.egg\pyparsing.py", line 1622, in parseString
  File "C:\Python36\lib\site-packages\pyparsing-2.2.0-py3.6.egg\pyparsing.py", line 1373, in _parseNoCache
  File "C:\Python36\lib\site-packages\pyparsing-2.2.0-py3.6.egg\pyparsing.py", line 1335, in preParse
TypeError: 'in <string>' requires string as left operand, not int
>>>

@bilderbuchi
Copy link
Author

I won't be at a PC to hunt this down for the next 2 weeks, but sounds like pyparsing is getting passed a number as integer, not converted to string, and chokes on that where there occurs a bla in somestring statement. Possibly a py2/3 behaviour change in ompython somewhere, maybe at the interfaces to external world. You can try to drop into the debugger when the exception gets thrwon, to see on what specific leftvalue it chokes.

@thorade
Copy link
Contributor

thorade commented Jun 16, 2017

@adeas31 I tried the same steps as shown in your script snippet and OMC crashes, independent of Python version.

@dietmarw
Copy link
Contributor

dietmarw commented Jun 16, 2017

I just tested the code snippet on a Ubuntu 16.04 64 bit installation with Python 3.5.2 and in my case it fails on:

>>> omc = OMCSessionZMQ()

https://gist.github.com/dietmarw/9a7128a06d4690992b214581a4dd0156

@thorade
Copy link
Contributor

thorade commented Jun 16, 2017

Same here. When runnign from IPython it just crashes, when running as a script from terminal, it also crashes, but it is possible to submit the core dump.

@adeas31
Copy link
Member

adeas31 commented Jun 16, 2017

Eeeehhhhh it seems the Linux version has issues. I will look into it.
Thanks for testing @thorade and @dietmarw

@dietmarw
Copy link
Contributor

Looks like also the Windows version has issues with this.
https://gist.github.com/dietmarw/864aae757f6590d96844fdb308c06b75

@adeas31
Copy link
Member

adeas31 commented Jun 16, 2017

Hmm that's weird. Can you send more information?
What is your current working directory?
What is the output of omc.exe -d=interactiveZMQ?

@adeas31
Copy link
Member

adeas31 commented Jun 16, 2017

The linux version is fixed in OpenModelica/OMCompiler#1686

@dietmarw
Copy link
Contributor

It looks like that omc.exe is not added to the path during the windows installation. Hence it will only work if you are inside %OPENMODELICA%\bin.

@dietmarw
Copy link
Contributor

Looks like there are more parsing errors hidden.

Using the simple example:

package foo
  model bar
    parameter Real foo= 1;
    Real bar;
  equation
    bar = 3*foo;
  end bar;
end foo;

I get the following error:

Python 3.6.1 |Continuum Analytics, Inc.| (default, May 11 2017, 13:25:24) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from OMPython import ModelicaSystem
>>> test = ModelicaSystem("foo.mo","foo.bar")
2017-06-16 15:31:50,364 - OMPython - INFO - OMC Server is up and running at file:///C:/Users/dietmarw/AppData/Local/Temp/openmodelica.port.56efc696438e497baf55567f55cbac35
'in <string>' requires string as left operand, not int
'in <string>' requires string as left operand, not int
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\dietmarw\AppData\Local\Continuum\Miniconda3\lib\site-packages\OMPython\__init__.py", line 656, in __init__
    self.__loadingModel(self.fileName_, self.modelName, self.lmodel)
  File "C:\Users\dietmarw\AppData\Local\Continuum\Miniconda3\lib\site-packages\OMPython\__init__.py", line 700, in __loadingModel
    self.getconn.sendExpression("setCommandLineOptions(\"+d=initialization\")")
  File "C:\Users\dietmarw\AppData\Local\Continuum\Miniconda3\lib\site-packages\OMPython\__init__.py", line 560, in sendExpression
    answer = OMTypedParser.parseString(result)
  File "C:\Users\dietmarw\AppData\Local\Continuum\Miniconda3\lib\site-packages\OMPython\OMTypedParser.py", line 82, in parseString
    return omcGrammar.parseString(string)[0]
  File "C:\Users\dietmarw\AppData\Local\Continuum\Miniconda3\lib\site-packages\pyparsing.py", line 1206, in parseString
    loc, tokens = self._parse( instring, 0 )
  File "C:\Users\dietmarw\AppData\Local\Continuum\Miniconda3\lib\site-packages\pyparsing.py", line 1066, in _parseNoCache
    preloc = self.preParse( instring, loc )
  File "C:\Users\dietmarw\AppData\Local\Continuum\Miniconda3\lib\site-packages\pyparsing.py", line 1028, in preParse
    while loc < instrlen and instring[loc] in wt:
TypeError: 'in <string>' requires string as left operand, not int

@thorade
Copy link
Contributor

thorade commented Jun 16, 2017

Does it work in other Python versions. e.g. in 2.7?
What is pip show pyparsing saying?

@adeas31
Copy link
Member

adeas31 commented Jun 16, 2017

It looks like that omc.exe is not added to the path during the windows installation. Hence it will only work if you are inside %OPENMODELICA%\bin.

No we need to set the environment for omc process. Should be fixed with #28

@dietmarw
Copy link
Contributor

dietmarw commented Jun 16, 2017

@thorade

(C:\Users\dietmarw\AppData\Local\Continuum\Miniconda3) C:\Users>pip show pyparsing
Name: pyparsing
Version: 2.1.4
Summary: Python parsing module
Home-page: http://pyparsing.wikispaces.com/
Author: Paul McGuire
Author-email: ptmcg@users.sourceforge.net
License: MIT License
Location: c:\users\dietmarw\appdata\local\continuum\miniconda3\lib\site-packages
Requires:

I have not tested it on Python 2 since that really is of no interest to me ;-)

@dietmarw
Copy link
Contributor

dietmarw commented Jun 16, 2017

I've just ran an upgrade and also pyparsing 2.2.0 has the same issue. I guess it's not pyparsing but what is handed to it by ModelicaSystem(). So basically there is still Python 3 incompatible code inside OMPython.

@dietmarw
Copy link
Contributor

I just ran pylint on OMPython .... not good ☹️

Running futurize on OMPython will give some fixes.

@adeas31
Copy link
Member

adeas31 commented Jun 16, 2017

#29 and #30 fixes the pyparsing errors. I got it working with python 3.6.1

Python 3.6.1 (v3.6.1:69c0db5, Mar 21 2017, 18:41:36) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from OMPython import OMCSessionZMQ
>>> omc = OMCSessionZMQ()
2017-06-16 17:08:59,429 - OMPython - INFO - OMC Server is up and running at file:///C:/Users/adeas31/AppData/Local/Temp/openmodelica.port.56a8bd4ae12940f8ac3516db1484320b
>>> omc.sendExpression('getVersion()')
'v1.12.0-dev-449-gf1f89cc (64-bit)'
>>> omc.sendExpression('quit()')
'quit requested, shutting server down\n'

@thorade
Copy link
Contributor

thorade commented Jun 16, 2017

I can confirm the simple example works on Windows with WinPython 2.7 and 3.4

@dietmarw For the fun of it, I ran the futurize script on OMPython, here is what it changed:
https://github.com/thorade/OMPython/pull/2/files
BUT: That breaks Py27 !! Not sure wat part exactly breaks it, though.

@dietmarw
Copy link
Contributor

Thanks @adeas31. I can confirm it works now under Windows. Will test it on Linux next week when the fixed nightlies are out.

@thorade Yes I got the same diff. But after all it was a different bug.

@bilderbuchi
Copy link
Author

It might be worth it to put all those things that are being tried out right now into a test suite so that it can easily be regression tested and also so that it's trivial to test different python versions and OSes.

@thorade
Copy link
Contributor

thorade commented Jun 17, 2017

It now works for me on Windows and Linux, Python 2.7, Python 3.4 and Python 3.5!!
When using the ZMQ interface, all dependencies can be installed using pip, no omniORB / omniidl required, installation is now much more convenient.

@thorade
Copy link
Contributor

thorade commented Jun 19, 2017

Even though the simple example works, some things have to be checked, the following things are not available in Py3:

@dietmarw
Copy link
Contributor

Just a heads up that the windows nightly of OpenModelica does not yet contain the latest OMPython version.

@dietmarw
Copy link
Contributor

dietmarw commented Jun 19, 2017

@thorade thorade mentioned this issue Jun 21, 2017
@dietmarw
Copy link
Contributor

I just found another function that still relies on OmniORB although I run under Python 3:

import OMPython
s = OMPython.OMCSession()
print(s.sendExpression("loadModel(Modelica)"))

Gives:

---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
<ipython-input-4-cdee912d9aeb> in <module>()
----> 1 s = OMPython.OMCSession()
      2 print(s.sendExpression("loadModel(Modelica)"))

/opt/conda/lib/python3.6/site-packages/OMPython/__init__.py in __init__(self, readonly)
    407         self._start_omc_process()
    408         # connect to the running omc instance using CORBA
--> 409         self._connect_to_omc()
    410 
    411     def __del__(self):

/opt/conda/lib/python3.6/site-packages/OMPython/__init__.py in _connect_to_omc(self)
    416         sys.path.append(os.path.join(self.omhome, 'lib', 'python'))
    417         # import the skeletons for the global module
--> 418         from omniORB import CORBA
    419         from OMPythonIDL import _OMCIDL
    420         # Locating and using the IOR

ModuleNotFoundError: No module named 'omniORB'

@thorade
Copy link
Contributor

thorade commented Jun 21, 2017

@dietmarw that is in the Corba Session, so it is supposed to be there.
There is OMCSessionBase and two classes that extend it, OMCSession for Corba and OMCSessionZMQfor ZMQ

@dietmarw
Copy link
Contributor

OK but what would be the proper "porcelain" command that one uses independent of what communication method is used?

@dietmarw
Copy link
Contributor

dietmarw commented Jun 21, 2017

Actually in order to not break existing scripts OMCSession should use the system that is working. So either CORBA or ZMQ. The user really should not have to rewrite their scripts for this.

@thorade
Copy link
Contributor

thorade commented Jun 21, 2017

@dietmarw So you suggest to rename current OMCSession to OMCSessionCorba and introduce a new class OMCSession that points to OMCSessionCorba or OMCSessionZMQ, depending on what was found!?

@adeas31
Copy link
Member

adeas31 commented Jun 21, 2017

We can't rely on "what was found". On Windows platform omniORB binaries are always available since we ship them.

I suggest we should use zeroMQ with OMCSession class, add a new class OMCSessionCorba and remove OMCSessionZMQ.

We could also completely get rid of CORBA since we have ZeroMQ now.

@sjoelund
Copy link
Member

sjoelund commented Jun 21, 2017 via email

@thorade
Copy link
Contributor

thorade commented Sep 27, 2017

The Windows installer for OpenModelica v1.13.0-dev-43-g6554061 (64-bit) still includes an old version of omniORB which is NOT Py3 compatible.
The file lib/python/omniORB/__init__.py has a print statement (without parantheses),
while the version 4.2.x available from sf.net has changed to a py2 & py3 compatible syntax.
https://sourceforge.net/p/omniorb/svn/HEAD/tree/branches/4_2/omniORBpy/python/omniORB/__init__.py

Could you please package the latest omniORB version with the OpenModelica installer for Windows?

@adeas31
Copy link
Member

adeas31 commented Sep 28, 2017

The whole idea of adding ZeroMQ is to avoid updating omniORB.

@adeas31
Copy link
Member

adeas31 commented Sep 28, 2017

@bilderbuchi I believe we can close this issue now?

@thorade
Copy link
Contributor

thorade commented Sep 28, 2017

Yes, you can close it, and then whoever finds a problem should open a new issue.

@bilderbuchi
Copy link
Author

Agreed, thank you everyone for the great work! 👏

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

No branches or pull requests

7 participants