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

Pyarrow hdfs support #224

Closed
wants to merge 30 commits into from

Conversation

Projects
None yet
4 participants
@harsham4026
Copy link
Contributor

commented Oct 1, 2018

No description provided.

Harsha Mandai added some commits Sep 27, 2018

Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
@todo

This comment has been minimized.

Copy link

commented Oct 1, 2018

verify what to raise

https://github.com/nteract/papermill/blob/111fcaaefa5425f43ee1ac4f871c89ec26688ecd/papermill/utils.py#L18-L23


This comment was generated by todo based on a TODO comment in 111fcaa in #224. cc @harsham4026.
@harsham4026

This comment has been minimized.

Copy link
Contributor Author

commented Oct 1, 2018

@MSeal made the changes for supporting the hdfs using pyarrow client. Please review the PR.

Harsha Mandai added some commits Oct 1, 2018

Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
@codecov

This comment has been minimized.

Copy link

commented Oct 1, 2018

Codecov Report

Merging #224 into master will decrease coverage by 1.05%.
The diff coverage is 68.83%.

@@            Coverage Diff             @@
##           master     #224      +/-   ##
==========================================
- Coverage   77.92%   76.87%   -1.06%     
==========================================
  Files          11       13       +2     
  Lines        1137     1202      +65     
==========================================
+ Hits          886      924      +38     
- Misses        251      278      +27
@harsham4026

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2018

@MSeal this build is getting failed for Python 3.7 because of from Cython.Distutils import build_ext as _build_ext
ModuleNotFoundError: No module named 'Cython'. I added Cython in dependencies though. Any suggestions to resolve this please?

Harsha Mandai added some commits Oct 2, 2018

Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai
Harsha Mandai Harsha Mandai

Harsha Mandai added some commits Oct 2, 2018

@MSeal

This comment has been minimized.

Copy link
Member

commented Oct 2, 2018

I might not get to looking at this until tomorrow. Two things to note, one we should avoid adding cython as a dependency if we can (complicates our build options heavily). Second this github issue seems to have some information on pyarrow in 3.7 builds: apache/arrow#1125

@MSeal

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

Posted an issue on pyarrow about this: apache/arrow#2708

We might need to restrict the feature to python 3.6 or less, or include it in a side package. I don't want cython and a bunch of build dependencies added if the wheel doesn't work out-of-the box.

@xhochy

This comment has been minimized.

Copy link

commented Oct 5, 2018

We will release wheels for pyarrow for all platforms. These then don't require Cython. As we hope to ship a new pyarrow release the next days with Python 3.7 wheels, I would suggest to just to review the code here and wait with the merge until pyarrow==0.11 is released. Then there is no need to restrict to Python < 3.7.

@MSeal

This comment has been minimized.

Copy link
Member

commented Oct 5, 2018

Thanks @xhochy , appreciate the input and quick response!

@rgbkrk

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

This PR has been open long enough that I'm going to recommend trying it again as a new PR, given how much has changed since October. Feel free to mention this PR in any new PRs related to this one.

Cheers!

@rgbkrk rgbkrk closed this Jun 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.