santoshphilip / eppy

scripting language for E+, Energyplus
MIT License
158 stars 66 forks source link

Issues when importing eppy in Ghpython editor of Grasshopper #147

Closed yiyuan1840 closed 7 years ago

yiyuan1840 commented 7 years ago

This post is cross-posted here I was trying to import eppy in the ghpython editor of Grasshopper, got the error:

Runtime error (InvalidOperationException): Unsupported param dictionary type: IronPython.Runtime.PythonDictionary
Traceback:
  line 44, in initpkg, "C:\Python27\Lib\site-packages\py\_apipkg.py"
  line 19, in <module>, "C:\Python27\Lib\site-packages\py\__init__.py"
  line 22, in <module>, "C:\Python27\Lib\site-packages\eppy\modeleditor.py"
  line 5, in script

The line 22 in my modeleditor.py is

from py._log import warning

I tried to comment line 22 and line 740 out in modeleditor.py, yet got another error:

Runtime error (ImportException): No module named multiprocessing
Traceback:
  line 23, in <module>, "C:\Python27\Lib\site-packages\eppy\runner\run_functions.py"
  line 28, in <module>, "C:\Python27\Lib\site-packages\eppy\modeleditor.py"
  line 5, in script
santoshphilip commented 7 years ago

let me see if I can get access to Grasshopper and recreate the problem. I have followed the discussion on unmethours

santoshphilip commented 7 years ago

I managed to get it running on Grasshopper in Rhino. Two lines have to commented out:

Line 22 in eppy/modeleditor.py
Line 23 in eppy/runner/runfunctions.py

Some functionality may not be available. Can you run it in this way and tell me what happens (I managed to crash Rhino - but that may be because I don't know that environment)

jamiebull1 commented 7 years ago

Seeing as when I run blame, both those lines come back as mine, I guess I should put some time into figuring out the problem. I don't have IronPython installed so none of the below is tested, but might work. I'll have a look at feasibility of adding IronPython to the CI testing tomorrow.

Anyway, a couple of possible fixes:

Line 22 in eppy/modeleditor.py from py._log import warning Maybe switch this for:

from logging import warning

and line 740 for:

            warning("The aname parameter should no longer be used.")

Line 23 in eppy/runner/runfunctions.py import multiprocessing as mp This is because IronPython has no support for multiprocessing. I suggest the best option here is to try/except ImportError on the multiprocessing import and then work around the use of multiprocessing in runIDFs by falling back to multirunner. Something like this:

    try:
        pool = mp.Pool(processors)
        pool.map(multirunner, processed_runs)
        pool.close()
    except NameError:
        map(multirunner, processed_runs)
santoshphilip commented 7 years ago

@jamiebull1 , You should add your name to modeleditor.py in the copyrights (at the top of the file). You have made substantial contributions to that file.

santoshphilip commented 7 years ago

@yiyuan1840, The issue has been fixed in the develop branch. We don't have access to the grasshopper environment. (not easily). Can you test it for us to see if it works.

yiyuan1840 commented 7 years ago

Looks like it is working well now! Thanks for the fix guys! @santoshphilip @jamiebull1 I will let you know if I met additional issues.

santoshphilip commented 7 years ago

@yiyuan1840 , I assume Rhino is not crashing now, when you run eppy

yiyuan1840 commented 7 years ago

@santoshphilip I have NOT seen any issues after the fix as of now. The crashing problem is resolved after the updates.

santoshphilip commented 7 years ago

thanks @yiyuan1840 I'll make a release with this version.

yiyuan1840 commented 7 years ago

Guys, I have switched to anaconda python from my system python, after updating to the develop branch of eppy under anaconda python library, some errors show up again. Any ideas why? This might be irrelevant to eppy, as I had witnessed the fix working well before I made the change to the system...

Runtime error (ArgumentTypeException): unicode_escape_decode() takes no arguments (1 given)
Traceback:
  line 14, in u, "C:\Anaconda2\Lib\site-packages\munch\python3_compat.py"
  line 337, in <module>, "c:\Anaconda2\Lib\site-packages\munch\__init__.py"
  line 18, in <module>, "C:\Anaconda2\Lib\site-packages\eppy\bunch_subclass.py"
  line 49, in <module>, "C:\Anaconda2\Lib\site-packages\eppy\EPlusInterfaceFunctions\iddindex.py"
  line 23, in <module>, "C:\Anaconda2\Lib\site-packages\eppy\EPlusInterfaceFunctions\parse_idd.py"
  line 14, in <module>, "C:\Anaconda2\Lib\site-packages\eppy\EPlusInterfaceFunctions\readidf.py"
  line 14, in <module>, "C:\Anaconda2\Lib\site-packages\eppy\idfreader.py"
  line 27, in <module>, "C:\Anaconda2\Lib\site-packages\eppy\modeleditor.py"
  line 6, in script
jamiebull1 commented 7 years ago

Looks like an munch bug. unicode_escape_decode definitely takes an argument as you can see if you try and call it without one.

>>>import codecs
>>>codecs.unicode_escape_decode()
Traceback (most recent call last):
  File "<input>", line 1, in <module>
TypeError: unicode_escape_decode() takes at least 1 argument (0 given)

I think the bug is related to the fact that all strings are unicode in IronPython, so the code in python3_compat.py is unnecessary.

Could you try changing the contents of your "C:\Anaconda2\Lib\site-packages\munch\python3_compat.py" to the following? If it works I'll raise a pull request with munch.

import sys

_PY2 = (sys.version_info < (3, 0))
_IRONPYTHON = platform.python_implementation == 'IronPython'

def identity(x):
    return x

# u('string') replaces the forwards-incompatible u'string'
if _PY2 and not _IRONPYTHON:
    import codecs

    def u(string):
        return codecs.unicode_escape_decode(string)[0]
else:
    u = identity

# dict.iteritems(), dict.iterkeys() is also incompatible
if _PY2:
    iteritems = dict.iteritems
    iterkeys = dict.iterkeys
else:
    iteritems = dict.items
    iterkeys = dict.keys
yiyuan1840 commented 7 years ago

Here are the results:

>>>import codecs
>>>codecs.unicode_escape_decode()
Runtime error (NotImplementedException): unicode_escape_decode
Traceback:
  line 15, in script

(line 15 is codecs.unicode_escape_decode())

Then I updated python3_compat.py with the code in your comment and do:

>>>import munch
Runtime error (ArgumentTypeException): unicode_escape_decode() takes no arguments (1 given)
Traceback:
  line 337, in <module>, "C:\Anaconda2\Lib\site-packages\munch\__init__.py"
  line 6, in script
  line 14, in u, "C:\Anaconda2\Lib\site-packages\munch\python3_compat.py"
jamiebull1 commented 7 years ago

So is it not picking up that you're running under IronPython? can you add a print(_IRONPYTHON) or step through in a debugger and check the value of _IRONPYTHON after the line: _IRONPYTHON = platform.python_implementation == 'IronPython'

And also maybe run platform.python_implementation == 'IronPython' from a python console.

yiyuan1840 commented 7 years ago

In the Ghpython editor, I did:

import platform
_IRONPYTHON = platform.python_implementation == 'IronPython'
print _IRONPYTHON

returned False

In the Python console, I did:

>>>import platform
>>> platform.python_implementation == 'IronPython'
False
jamiebull1 commented 7 years ago

Can you try printing the platform.python_implementation?

yiyuan1840 commented 7 years ago
>>> print platform.python_implementation
<function python_implementation at 0x0000000002A043C8>
jamiebull1 commented 7 years ago

Haha, ok. Try again with closing brackets. I thought it was an attribute but it's a function.

print platform.python_implementation()

yiyuan1840 commented 7 years ago

OK, sure.

>>> print platform.python_implementation()
CPython
jamiebull1 commented 7 years ago

So it's not running with IronPython? I think grasshopper requires IronPython so that might be your problem.

I'm pretty sure you can't get IronPython under Anaconda either.

El jue., 16 mar. 2017 15:39, Yiyuan Jia notifications@github.com escribió:

OK, sure.

print platform.python_implementation() CPython

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/santoshphilip/eppy/issues/147#issuecomment-287077700, or mute the thread https://github.com/notifications/unsubscribe-auth/ADo-yGnXgKKvGz29WdPfPTDNzqBcqxzUks5rmUmIgaJpZM4MZ0ZQ .

yiyuan1840 commented 7 years ago

I guess I was using Anaconda Python for the test:

(C:\Anaconda2) C:\Users\YJIA>python
Python 2.7.13 |Anaconda 4.3.1 (64-bit)| (default, Dec 19 2016, 13:29:36) [MSC v.1500 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
Anaconda is brought to you by Continuum Analytics.
Please check out: http://continuum.io/thanks and https://anaconda.org
>>> import platform
>>> print platform.python_implementation()
CPython
>>>
yiyuan1840 commented 7 years ago

Thanks to @dglassman, the bug is fixed inside munch library. Basically, the conflicts are in python3_compat.py. "The issue is that munch includes a shim for compatibility with Python3, but IronPython doesn't understand it. And since we're working with Python 2.7, we don't need it." (@dglassman)

The line 14 of the original python3_compat.py was: return codecs.unicode_escape_decode(string)[0]

Is replaced with: return unicode(string)

And that fixed the errors!

santoshphilip commented 7 years ago

reopening until resolution.

jamiebull1 commented 7 years ago

It's resolved. The bug was in munch (see above). Suggest we change the minimum version of munch in setup.py but that's all.

santoshphilip commented 7 years ago

If I understand this right

jamiebull1 commented 7 years ago

If I've understood right, it was fixed today in munch thanks to @yiyuan1840 raising an issue there. So whatever the new version number is should be what we require as minimum in setup.py.

santoshphilip commented 7 years ago

@yiyuan1840 , I curious how this got resolved. Did you open an issue in munch -> https://github.com/Infinidat/munch (I was not able to locate this issue there.) or was it already fixed in the new version of munch (was released a few days back)

Some of this confusion is happening since we are trying to resolve this without having IronPython installed. So we are depending on you for this :-)

yiyuan1840 commented 7 years ago

@santoshphilip @jamiebull1 Sorry for the confusion. It is not fixed by munch, I have to manually change the code in python3_compat.py to get it work.

I think this might be what has happened: since I installed the newest version of Anaconda, the munch library has also been upgraded, the issue in python3_compat.py (complained by IronPython) raised after the updates. It was working OK in my previous python setups, which had an older version of munch.

I would open an issue in munch.

jamiebull1 commented 7 years ago

@yiyuan1840 You could just raise a pull request with them as you've already fixed it.

@santoshphilip Perhaps we need to patch munch when we import it until they make the fix?

yiyuan1840 commented 7 years ago

Also, if you want to test IronPython and Grasshopper, there's a 90-day evaluation period for Rhino, and you can install Grasshopper(plugin for Rhino) and GhPython(plugin for Grasshopper) freely.

jamiebull1 commented 7 years ago

I'd be interested in any way we could add it to our continuous integration process, but not at all in testing it manually. I'd rather say it's not guaranteed to be supported since we don't have an automated way to check whether changes made in eppy break under IronPython.

El vie., 17 mar. 2017 15:18, Yiyuan Jia notifications@github.com escribió:

Also, if you want to test IronPython and Grasshopper, there's a 90-day evaluation period for Rhino, and you install Grasshopper(plugin for Rhino) and GhPython(plugin for Grasshopper) freely.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/santoshphilip/eppy/issues/147#issuecomment-287366049, or mute the thread https://github.com/notifications/unsubscribe-auth/ADo-yKAjCbT2FNmAXRShnbW7JWJr1Xcfks5rmpY4gaJpZM4MZ0ZQ .

santoshphilip commented 7 years ago

@yiyuan1840 , I agree with @jamiebull1 about you making a pull request. In case you have never done this before, here is a link that talks about it https://yangsu.github.io/pull-request-tutorial/

santoshphilip commented 7 years ago

@jamiebull1 , I am inclined not to make a patch and just wait for a fix to munch. Unless there is a huge demand for it :-)

santoshphilip commented 7 years ago

regarding continuous integration we have the following python implementations:

I know that eppy is used with IronPython and maybe withJython. @eayoungs wrote code to ensure that numpy was not a dependency and so eppy would likely run on both those implementations. Extra effort has been put in to ensure that the core functionality of eppy is in pure python. (except in drawing the loop diagrams and in the E+ runner)

jamiebull1 commented 7 years ago

The main option for CI under Python implementations other than CPython is Jenkins. We'd need to provide our own Jenkins server for that though, rather than having the free AppVeyor and TravisCI services. I can take at look at hosting a CI server on Heroku maybe. I have a few spare dynos.

yiyuan1840 commented 7 years ago

Looks like the bug with munch has been fixed in this issue. And I just tested Gh with the new munch, it worked well. I'm working with Grasshopper and eppy with the hope to develop some re-useable modules(interface for eppy) in creating energy models. I'm not sure how that is in alignment with the eppy development. If IronPython is no longer supported then this path is dead. In that sense what do you think would be a good interface environment for eppy (in term of geometry and systems creation), Dynamo might be another option, while a couple of months ago I heard python is not supported well in the dynamo, while it might not be the case as of now.

santoshphilip commented 7 years ago

@jamiebull1 , I went to a talk yesterday and came across some interesting ideas. The talk was "Testing in Layers" by Alex Martelli. (he is the author of "Python in a Nutshell")

Alex told an interesting story about the release of python 3.6.1rc1. As they were heading towards the release date, the lead maintainer asked the contributors to really test their contributions before pushing it into github. The continous integration tests was being turned off.

Sounds like an outrageous thing to do :-) Apparently they had only so much free service from Travis and appveyor. The tests were taking too long - each time a push came into the repository, the tests were getting backed up. At this point the tests were running into days. No way they would make their deadline. (No particular moral in the story that applies to us in eppy :-)

In any case the slides for the talk are at: http://www.aleax.it/pyconit17_en.pdf He is planning to give present this at the Pycon Italy. I found his thinking very useful for some of the other projects I am working on. It does apply to eppy too, but not as critical.

Coming back to our issue at hand:

  1. Does eppy work on other python implementations (ironpython, jython etc)
  2. How do we set up continuous testing for this.
  3. Should we set up continuous testing for this.

Let us give item 2 and 3 a lower priority.

Regarding 1. Does eppy work on other python implementations (ironpython, jython etc), I am planning to tap into the wisdom of the crowd. In practical terms, hoping to persuade @yiyuan1840 to run all the tests on IronPython. I'll also try to find someone to test it on Jython.

santoshphilip commented 7 years ago

@yiyuan1840, Thanks for following up on this. Can you confirm that eppy works with a clean install from the bugfix branch of munch.

@jamiebull1, I cloned munch and looked into it.

I assume that at one point they will release a version with the bugfix. I am watching that repository now. So I'll get a notification when they make a release.

In the meantime, I am reluctant to make a release that depends on a bugfix branch. They may delete the branch and our release will break.

Any thoughts ? I am OK with just waiting on it

santoshphilip commented 7 years ago

@yiyuan1840 , responding further to your comments.

IronPython is supported. I want your help in making that possible. Right now I am not sure everything in eppy runs on IronPython. To ensure this we have to confirm that all the tests for eppy pass on IronPython. I can help you with this if you choose to take it on (sounds like the line form "Mission impossible", but this is much easier)

Let me know.

santoshphilip commented 7 years ago

@yiyuan1840, some further thoughts, since you are thinking of working on reusable modules with eppy.

eppy is designed to build other modules. So you are on the right track. If you build some really useful modules, let us talk about integrating it into eppy.

yiyuan1840 commented 7 years ago

@santoshphilip I think the fix has been committed to the master branch of munch. I had tried to uninstall and reinstalled munch (the master branch I suppose) from the command line, it works well in my little grasshopper-eppy test module (which calls eppy to extract zone area and volume from a given IDF file)

Yeah, I could take on the task to test eppy in IronPython. I guess there is a need to develop some standardized "test workflow" for the quality assurance purpose, does that sounds about right?

santoshphilip commented 7 years ago

@yiyuan1840 , In our usual testing we just run py.test in the command line

It will run all the unit tests - and show us where they pass or fail

Can you open a new issue. Call it "testing for IronPython". Assign it to yourself. We can continue this conversation there.

santoshphilip commented 7 years ago

@yiyuan1840 , You are right about the munch release. The latest release has the bugfix. I'll make a new release of eppy to reflect this change.

santoshphilip commented 7 years ago

@yiyuan1840 , regarding testing, you can run it as py.test OR python -m pytest

santoshphilip commented 7 years ago

take a look at issue #162 . There is a possible solution to this

santoshphilip commented 7 years ago

this has been resolved issue #162 I am going to close it here. #162 is open for the moment