search

roualdes / bridgestan

BridgeStan provides efficient in-memory access through Python, Julia, and R to the methods of a Stan model.
https://roualdes.github.io/bridgestan
BSD 3-Clause "New" or "Revised" License
88 stars 12 forks source link

Fix: Windows TBB path logic fails if BridgeStan is not yet downloaded #149

Closed WardBrian closed 1 year ago

WardBrian commented 1 year ago

Closes #148

This also adds a very trivial test: Can the package be imported without BRIDGESTAN being set ahead of time.

WardBrian commented 1 year ago

I'd like to publish a 2.1.1 shortly after this, assuming nothing new comes in

aseyboldt commented 1 year ago

Seems I can't easily test this PR directly in the nutpie CI, because checking out stan fails with a "path too long" error... https://github.com/pymc-devs/nutpie/actions/runs/5424318833/jobs/9863510219?pr=42#step:8:32

But this looks good to me, assuming tests pass.

WardBrian commented 1 year ago

fails with a "path too long" error...

Oh man, I recently dealt with this for Facebook's prophet package. Pip seems to like really long base path names, and a lot of the bash-on-Windows tools (including git, but also things like mingw32-make), do not respect the LONG_PATHS setting and fail if the path is over ~280 characters. Super annoying

You may be able to get things going by setting %TEMPDIR% to somewhere in the top-level of C:\, but that might still be too long. Git cloning it somewhere and then pip install -e . in that location would probably work.

WardBrian commented 1 year ago

@roualdes thoughts on this fix and a .1 release?

aseyboldt commented 1 year ago

This does indeed fix the issue. I had to call the setup function manually though, loading the library failed without it.

WardBrian commented 1 year ago

Calling the setup function manually seems like a reasonable requirement for users who intend to load the dll not using the Python class we provide