eggplantbren / DNest4

Diffusive Nested Sampling
MIT License
60 stars 21 forks source link

Fixing Python extension and build process #33

Closed dfm closed 4 years ago

dfm commented 4 years ago

The Python extension had fallen out of date and this pull request updates all this to work again.

eggplantbren commented 4 years ago

Thanks heaps for doing this. I have successfully reinstalled it on Ubuntu 18.04 and it worked fine. It's not working on Manjaro but that's a Manjaro issue that I've encountered before and need to work around.

I'm having trouble using --without-cext on Ubuntu 18.04. I get this error

brewer@sc402854:~/Projects/DNest4$ apython setup.py install --without-cext
Traceback (most recent call last):
  File "setup.py", line 100, in <module>
    sys.path.insert(0, os.path.join(basedir, "python"))
NameError: name 'basedir' is not defined

I'll probably just accept the PR if you can fix this particular problem. Then I'll have to talk to you to understand what you did and what I should be doing in the future.

dfm commented 4 years ago

I fixed that bug, but wait a bit to merge - there's one other thing that I'm trying first to help Geraint.

The main change that was needed was in _dnest4.pyx to add the _adaptive parameter in the Sampler constructor. The rest of it is really just quality of life improvements and things to get it to work on pip and conda which it now does.

eggplantbren commented 4 years ago

I'll wait for your approval before accepting or even retrying. I did know I had to update some things in the Python extension when I added new files but I forgot when I added the adaptive thing (which isn't a great feature, don't worry about that). Oops

On Thu, Oct 24, 2019 at 8:02 AM Dan Foreman-Mackey notifications@github.com wrote:

I fixed that bug, but wait a bit to merge - there's one other thing that I'm trying first to help Geraint.

The main change that was needed was in _dnest4.pyx to add the _adaptive parameter in the Sampler constructor. The rest of it is really just quality of life improvements and things to get it to work on pip and conda which it now does.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

-- Dr Brendon J. Brewer Department of Statistics, The University of Auckland, New Zealand Ph: +64 27 500 1336 Web: https://www.brendonbrewer.com/

dfm commented 4 years ago

I think this should be all set to go now. I bumped the Python version to 0.2.4 so maybe it's time for a full release across the package too. When you have an update that affects the Python, let me know and I can easily push it to pip and conda.

eggplantbren commented 4 years ago

Sorry, just noticed this after merging. I have lots of usage of

import dnest4.classic as dn4

and

import dnest4.utils

in my projects. Are you able to fix up the __init__.py (I think that's what matters) so these will work again?

dfm commented 4 years ago

both of those work just fine!

eggplantbren commented 4 years ago

You're right. It's PEBKAC on my end.

The Manjaro problem led me to use --without-cext but dnest4.utils depends on the Cython. I'll need to rediscover my other workaround.

On Thu, Oct 24, 2019 at 12:59 PM Dan Foreman-Mackey < notifications@github.com> wrote:

both of those work just fine!

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/eggplantbren/DNest4/pull/33?email_source=notifications&email_token=AAMBKOQYJS6XG7AI246YV7DQQDQPHA5CNFSM4JEB7FH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECDHZIQ#issuecomment-545684642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMBKOS6ZV7FXRPVPYJ6Z7LQQDQPHANCNFSM4JEB7FHQ .

-- Dr Brendon J. Brewer Department of Statistics, The University of Auckland, New Zealand Ph: +64 27 500 1336 Web: https://www.brendonbrewer.com/

eggplantbren commented 4 years ago

No need - I just used your pip build. Woohoo :)

On Thu, Oct 24, 2019 at 1:03 PM Brendon Brewer brendon.brewer@gmail.com wrote:

You're right. It's PEBKAC on my end.

The Manjaro problem led me to use --without-cext but dnest4.utils depends on the Cython. I'll need to rediscover my other workaround.

On Thu, Oct 24, 2019 at 12:59 PM Dan Foreman-Mackey < notifications@github.com> wrote:

both of those work just fine!

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/eggplantbren/DNest4/pull/33?email_source=notifications&email_token=AAMBKOQYJS6XG7AI246YV7DQQDQPHA5CNFSM4JEB7FH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECDHZIQ#issuecomment-545684642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMBKOS6ZV7FXRPVPYJ6Z7LQQDQPHANCNFSM4JEB7FHQ .

-- Dr Brendon J. Brewer Department of Statistics, The University of Auckland, New Zealand Ph: +64 27 500 1336 Web: https://www.brendonbrewer.com/

-- Dr Brendon J. Brewer Department of Statistics, The University of Auckland, New Zealand Ph: +64 27 500 1336 Web: https://www.brendonbrewer.com/

dfm commented 4 years ago

Awesome! I'm glad that worked!!