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

ARROW-2976: [Python] Fix pyarrow.get_library_dirs #2398

Closed
wants to merge 2 commits into from

Conversation

@pcmoritz
Copy link
Contributor

pcmoritz commented Aug 7, 2018

No description provided.

pcmoritz added 2 commits Aug 7, 2018
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Aug 7, 2018

Codecov Report

Merging #2398 into master will increase coverage by 2.11%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2398      +/-   ##
==========================================
+ Coverage   84.73%   86.84%   +2.11%     
==========================================
  Files         293      237      -56     
  Lines       45471    42719    -2752     
==========================================
- Hits        38529    37099    -1430     
+ Misses       6905     5620    -1285     
+ Partials       37        0      -37
Impacted Files Coverage Δ
python/pyarrow/__init__.py 52.11% <0%> (-12.8%) ⬇️
go/arrow/math/float64_amd64.go
go/arrow/type_string.go
go/arrow/array/bufferbuilder_numeric.gen.go
go/arrow/array/numericbuilder.gen.go
rust/src/buffer.rs
rust/src/list_builder.rs
go/arrow/internal/cpu/cpu_x86.go
go/arrow/internal/bitutil/bitutil.go
rust/src/list.rs
... and 47 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91ffc00...6d88fcd. Read the comment docs.

@pcmoritz

This comment has been minimized.

Copy link
Contributor Author

pcmoritz commented Aug 8, 2018

@wesm @pitrou Can one of you have a quick look if that seems to be the right way to do this? It's working for me but there is probably lots of corner cases.

@pitrou

This comment has been minimized.

Copy link
Contributor

pitrou commented Aug 8, 2018

Can one of you have a quick look if that seems to be the right way to do this?

No idea, sorry. I'm a bit surprised the CMake scripts aren't able to do that by themselves?
Apparently LLVM installs some CMake scripts that allow third-party projects to include the necessary definitions: https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project
(not that I find CMake easy to work with ;-))

@wesm

This comment has been minimized.

Copy link
Member

wesm commented Aug 8, 2018

I think this is for 3rd party libraries that aren't using cmake? (NumPy has some functions like this)

I tested this locally where I have the libraries installed somewhere else and it seems to be working:

In [1]: import pyarrow
pya
In [2]: pyarrow.get_library_dirs()
Out[2]: ['/home/wesm/code/arrow/python/pyarrow', '/home/wesm/local/lib']
@wesm
wesm approved these changes Aug 8, 2018
Copy link
Member

wesm left a comment

+1

@wesm wesm closed this in 0cef55a Aug 8, 2018
@wesm wesm deleted the pcmoritz:fix-get-library-dirs branch Aug 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.