Skip to content

Recipe for Python part of Apache Arrow - #1732

Merged
ocefpaf merged 1 commit into
conda-forge:masterfrom
wesm:pyarrow
Oct 19, 2016
Merged

Recipe for Python part of Apache Arrow#1732
ocefpaf merged 1 commit into
conda-forge:masterfrom
wesm:pyarrow

Conversation

@wesm

@wesm wesm commented Oct 7, 2016

Copy link
Copy Markdown
Member

Requires #1729 and #992

@conda-forge-linter

Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/pyarrow) and found some lint.

Here's what I've got...

For recipes/pyarrow:

  • Failed to even lint the recipe (might be a conda-smithy bug) 😢

Comment thread recipes/pyarrow/meta.yaml Outdated
version: {{ version }}

source:
git_url: https://github.com/apache/arrow.git

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the feedback from @ocefpaf on arrow-cpp here also apply?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. If you could make those changes it will make this clearer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, will wait for #992 to become feedstock then get this to a green build

@chrisaycock

Copy link
Copy Markdown

The build and dependencies LGTM.

@wesm
wesm force-pushed the pyarrow branch 5 times, most recently from daaffca to 0b7593d Compare October 10, 2016 23:48
@wesm

wesm commented Oct 11, 2016

Copy link
Copy Markdown
Member Author

This is failing because of PARQUET-747. I will update the parquet-cpp build and report back when this build is green

@chrisaycock

Copy link
Copy Markdown

@wesm

wesm commented Oct 11, 2016

Copy link
Copy Markdown
Member Author

OK, the parquet-cpp conda-forge package has been updated, looks like we're good to go whenever the OS X build passes

Comment thread recipes/pyarrow/meta.yaml Outdated
number: 0
rpaths:
- lib # [unix]
- lib/python{{environ.get('PY_VER')}}/site-packages/pyarrow # [unix]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linter is failing here. Would {{ environ['PY_VER'] + '*' }} work instead? That is known to pass.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@conda-forge-linter

Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pyarrow) and found it was in an excellent condition.

@wesm

wesm commented Oct 11, 2016

Copy link
Copy Markdown
Member Author

@ocefpaf nope, that broke my rpath. trying again without the '*'

@ocefpaf

ocefpaf commented Oct 11, 2016

Copy link
Copy Markdown
Member

@ocefpaf nope, that broke my rpath. trying again without the '*'

No problem. We should probably fix the linter and not the recipe anyways.

@wesm

wesm commented Oct 12, 2016

Copy link
Copy Markdown
Member Author

This is good to go once the build is green

@wesm

wesm commented Oct 12, 2016

Copy link
Copy Markdown
Member Author

Argh, well we have an rpath issue on OS X now. Looking.

@wesm

wesm commented Oct 12, 2016

Copy link
Copy Markdown
Member Author

@ocefpaf I'm pretty bewildered by the OS X rpath stuff, do you have any advice? Our package builds on our Travis CI are working for OS X, e.g. https://travis-ci.org/apache/arrow/jobs/166942076

The issue here is that there is a C++ shared library nested under pyarrow/ and this is being hidden (?) by the install_name_tool. I can't tell what's different about the conda forge environment that would make this fail. It's XCode 6.0 versus 6.1, but not sure if that's enough to screw things up.

@ocefpaf

ocefpaf commented Oct 12, 2016

Copy link
Copy Markdown
Member

@ocefpaf I'm pretty bewildered by the OS X rpath stuff, do you have any advice?

Sorry, never used a Mac in my life. Maybe @msarahan can help here.

Our package builds on our Travis CI are working for OS X, e.g. https://travis-ci.org/apache/arrow/jobs/166942076

Are you building with conda there? Note that the xcode is different too, but I don't think that is the issue.

The issue here is that there is a C++ shared library nested under pyarrow/ and this is being hidden (?) by the install_name_tool. I can't tell what's different about the conda forge environment that would make this fail. It's XCode 6.0 versus 6.1, but not sure if that's enough to screw things up.

I maintain this recipe where we had to manually change paths with install_name_tool. Maybe that is the case here?

@wesm

wesm commented Oct 12, 2016

Copy link
Copy Markdown
Member Author

I'm trying to resolve this in apache/arrow#171, setting this to build from that PR

@ocefpaf

ocefpaf commented Oct 12, 2016

Copy link
Copy Markdown
Member

@wesm Linux is OK and you are already skipping Windows. Do you want to skip OS X for now, get this into a feedstock, and experiment there? It would make your tests more agile and you would free AppVeyor's queue. (Even when skipping AppVeyor will start. You can force skip it by adding skip appveyor in the commit message though.)

@wesm

wesm commented Oct 12, 2016

Copy link
Copy Markdown
Member Author

Let me see if this commit fixed it, and if not I'll make this Linux only to go to feedstock

@wesm

wesm commented Oct 17, 2016

Copy link
Copy Markdown
Member Author

@msarahan @ocefpaf @jakirkham this problem appears to be localized to the old version of XCode being used in conda-forge -- I left a comment on the bigger discussion about OS X architecture

@wesm

wesm commented Oct 19, 2016

Copy link
Copy Markdown
Member Author

@ocefpaf I disabled all but Linux for now, I can re-enable OS X when the XCode toolchain issues are resolved. thanks!

@ocefpaf

ocefpaf commented Oct 19, 2016

Copy link
Copy Markdown
Member

Linux is no longer 😒

import: 'pyarrow'
Traceback (most recent call last):
  File "/staged-recipes/build_artefacts/test-tmp_dir/run_test.py", line 26, in <module>
    import pyarrow
  File "/opt/conda/envs/_test/lib/python3.5/site-packages/pyarrow/__init__.py", line 20, in <module>
    import pyarrow.config
ImportError: /opt/conda/envs/_test/lib/python3.5/site-packages/pyarrow/../../../libparquet_arrow.so: undefined symbol: _ZN5arrow16PrimitiveBuilderINS_11BooleanTypeEE6FinishEv
TESTS FAILED: pyarrow-0.1.post-0

@wesm

wesm commented Oct 19, 2016

Copy link
Copy Markdown
Member Author

Visibility is such a pain! I will fix

@ocefpaf

ocefpaf commented Oct 19, 2016

Copy link
Copy Markdown
Member

I can re-enable OS X when the XCode

@wesm this kind of changes needs more discussion. In principle I am 👍 to that but we need to consensus as this change should happen everywhere. (In fact, for my local builds, I use homebrew's gcc so I am not tied to OS X specific stuff.)

It would be nice if you, or someone in your group, could participate some of our meetings and request that item to be discussed. See https://conda-forge.hackpad.com/conda-forge-meetings-2YkV96cvxPG

Maybe we could start by putting together a conda-forge-enhancement-proposal to move to a modern XCode. See https://github.com/conda-forge/conda-forge-enhancement-proposals

@wesm

wesm commented Oct 19, 2016

Copy link
Copy Markdown
Member Author

Oh, actually this is because the parquet-cpp package is out of date, whoops.

…ds but

Linux until OS X toolchain issues resolved
@wesm

wesm commented Oct 19, 2016

Copy link
Copy Markdown
Member Author

@ocefpaf looks like something is broken in the conda forge toolchain now?

@ocefpaf

ocefpaf commented Oct 19, 2016

Copy link
Copy Markdown
Member

@ocefpaf looks like something is broken in the conda forge toolchain now?

Good old "turn it off and on again" seems to fix it 😄

I re-started CircleCI and cancelled AppVeyor and Travis-CI since you are only building Linux for now.

@ocefpaf
ocefpaf merged commit 90eba6c into conda-forge:master Oct 19, 2016
@wesm

wesm commented Oct 19, 2016

Copy link
Copy Markdown
Member Author

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants