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

Update setting of dlopenflags() to current Python standards (fixes #632) #634

Merged
merged 3 commits into from Jan 26, 2017

Conversation

@heplesser
Copy link
Contributor

heplesser commented Jan 19, 2017

This solves Python 3.6 compatibility problems (#632) and removes code deprecated since Python 2.6.

…3.6 compatibility problems.
@heplesser
Copy link
Contributor Author

heplesser commented Jan 19, 2017

@Silmathoron Could you test this with Python 3.6? I have only tested 3.5 and 2.7 on OSX, while Travis will test 2.7 under Linux.

@heplesser
Copy link
Contributor Author

heplesser commented Jan 19, 2017

Hm, seems that at least Ubuntu 14.04 does not provide RTLD_NOW in ctypes, and that importing pynestkernel fails if it is not set. So we may need to import DLFCN. I will update the PR tomorrow.

Kari Kode and others added 2 commits Jan 20, 2017
Kari Kode
Copy link
Member

Silmathoron left a comment

All good under 3.6 also 👍

@heplesser
Copy link
Contributor Author

heplesser commented Jan 20, 2017

@Silmathoron I just (about five minutes ago) pushed some fixes. Is your 3.6 approval with those fixes, or was that based on my push from about 15 hours ago?

@Silmathoron
Copy link
Member

Silmathoron commented Jan 20, 2017

Yes, don't worry ;)

@terhorstd terhorstd requested review from DimitriPlotnikov and removed request for apeyser Jan 23, 2017
@DimitriPlotnikov
Copy link

DimitriPlotnikov commented Jan 24, 2017

Dear @Silmathoron and @heplesser,

I have been working quite a bit on the PR and corresponding issue. Now, I'm a bit confused what is the actual problem been solved in there.

I'll explain what did I try. In a clean Ubuntu 16.4.1LTS with python 2.7 and 3.5 I tried:

Building nest with python3 flags (-Dwith-python=3)
nest-simulator-master: python2 doesn't work with nest. python3 interpreter works with nest.
nest-simulator-PR: python2 doesn't work with nest. python3 interpreter works with nest.

Building nest without python 3 flags (-Dwith-python=3)
nest-simulator-master: python2 works with nest. python3 doesn't work with nest (problem mentioned in the issue).
nest-simulator-PR: python2 works with nest. python3 works with nest.

In my opinion It would be helpful to complement the issue with the information about:
a) Which software version were used
b) How nest-simulator were build
c) How to reproduce the error

@Silmathoron
Copy link
Member

Silmathoron commented Jan 24, 2017

Hi @DimitriPlotnikov!
Unfortunately I'm a bit unsure about what your problem is (and I think there are unfortunate copy-paste in your comment ^^")

Just to make sure everything is clear:

  • using -Dwith-python=X will install PyNEST for python X.Y (with highest available Y) only, e.g. -Dwith-python=2 will most probably install it in the path of python 2.7, meaning that any python 3.Y on your machine won't be able to use it (and the reverse is true if you use -Dwith-python=3
  • the actual problem with the previous __init__.py was that, for python 3.6 (i.e. installing with -Dwith-python=3 and python 3.6 available on your machine), the import failed.

Thus, HEP's changes make sure that this problem is corrected:

  1. sys.setdlopenflags(os.RTLD_NOW | os.RTLD_GLOBAL) works from 3.3 on
  2. sys.setdlopenflags(dl.RTLD_GLOBAL | dl.RTLD_NOW) should work for 2.6--3.2 on most platforms
  3. if not, one of sys.setdlopenflags(DLFCN.RTLD_GLOBAL | DLFCN.RTLD_NOW) or sys.setdlopenflags(ctypes.RTLD_GLOBAL | ctypes.RTLD_NOW) should do the trick
  4. unless we're on a mac, then we need sys.setdlopenflags(ctypes.RTLD_GLOBAL)
@DimitriPlotnikov
Copy link

DimitriPlotnikov commented Jan 24, 2017

@Silmathoron

You provided me the information which I more or less wanted to know. I'll conduct a test with python 3.6

And no, there is no copy&paste errors in my previous message. I just summarized my observations and tests which I done.

@Silmathoron
Copy link
Member

Silmathoron commented Jan 24, 2017

Building nest without python 3 flags (-Dwith-python=3)

Looks highly suspect to me, though... but ok, let us know ;)

@DimitriPlotnikov
Copy link

DimitriPlotnikov commented Jan 24, 2017

From: https://github.com/nest/nest-simulator/blob/master/INSTALL

-Dwith-python=[OFF|ON|2|3]                   Build PyNEST. To set a specific Python version, set 2 or 3. [default=ON]

Since there is no information regarding such options in the PR and issue I tried to reconstruct it from the docu. If I did it wrong, please, let me known.

@Silmathoron
Copy link
Member

Silmathoron commented Jan 24, 2017

Sorry, I should have made it clearer: I just meant I think you used 3 when you wanted to use 2.
I would expect

Building nest without python 3 flags (-Dwith-python=2)

But as I said, I think it's a copy-paste error since you seem to understand the option without any problem ;)

@heplesser
Copy link
Contributor Author

heplesser commented Jan 26, 2017

@DimitriPlotnikov Did your test with Python 3.6 go well?

@DimitriPlotnikov
Copy link

DimitriPlotnikov commented Jan 26, 2017

@heplesser
I'm still actively testing the PR in different configurations. Update the PR and issue today during the day.

@DimitriPlotnikov
Copy link

DimitriPlotnikov commented Jan 26, 2017

Tested the PR in a Ubuntu 16.10 instance with following python version: 2.7.12, 3.5, 3.6. I was always able to import nest in every of the mentioned python interpreters.

@heplesser
Copy link
Contributor Author

heplesser commented Jan 26, 2017

Summary: NEST now works with Python 3.6.

@heplesser heplesser merged commit 4a2938c into nest:master Jan 26, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@heplesser heplesser deleted the heplesser:fix-632-RTLD branch Mar 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.