Add datafusion-python #69

jorgecarleitao · 2021-04-25T16:03:01Z

This is a PR with the source code of python-datafusion, currently available at https://github.com/jorgecarleitao/datafusion-python and released in pypi as datafusion.

The goal of this PR is to gauge interest of moving that code base closer to datafusion and to within ASF.

Some notes:

The documentation is lacking, but I would hope to have the docs published somewhere via readthedocs.
The release builds wheels for windows, mac, and manylinux2010, which cover some of the user-base

codecov-commenter · 2021-04-25T16:19:16Z

Codecov Report

Merging #69 (3d9e1a3) into master (2423ff0) will decrease coverage by 0.66%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
- Coverage   76.43%   75.77%   -0.67%     
==========================================
  Files         135      142       +7     
  Lines       23264    23467     +203     
==========================================
  Hits        17782    17782              
- Misses       5482     5685     +203

Impacted Files	Coverage Δ
python/src/context.rs	`0.00% <0.00%> (ø)`
python/src/dataframe.rs	`0.00% <0.00%> (ø)`
python/src/errors.rs	`0.00% <0.00%> (ø)`
python/src/expression.rs	`0.00% <0.00%> (ø)`
python/src/scalar.rs	`0.00% <0.00%> (ø)`
python/src/types.rs	`0.00% <0.00%> (ø)`
python/src/udaf.rs	`0.00% <0.00%> (ø)`
... and 4 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 2423ff0...3d9e1a3. Read the comment docs.

python/Cargo.toml

python/LICENSE

andygrove · 2021-04-25T16:25:08Z

Thank you @jorgecarleitao I am really excited to see this and would love to see this merged into arrow-datafusion.

jorgecarleitao · 2021-04-25T17:07:23Z

Some notes:

this points to a rather old commit in DataFusion. We need to work on that
the CI did not run for some reason; I need to fix that
I need to push the Apache license and headers to flag that this is being licensed to ASF via Apache 2 and not MIT

xhochy · 2021-04-26T12:08:13Z

.github/workflows/python_build.yml

@@ -0,0 +1,72 @@
+name: Build


The tag release probably won't work in the context of an ASF repo anymore?

Yeap, we will need to work out a packaging; the build of the wheels is imo still relevant, as it is not so easy in Rust (afai understood support for this is still a bit WIP). Building the manylinux was a feat.

.github/workflows/python_test.yaml

xhochy · 2021-04-26T12:11:15Z

python/README.md

+
+```bash
+pip install datafusion
+```


Adding here as a suggestion but I'll take a look at packaging it as a conda package. I'll cc you on the PR once I got a bit working.

Suggested change

```

or via conda/mamba:

conda install -c conda-forge datafusion mamba install -c conda-forge datafusion

@xhochy
If you want you can ping me on the staged-recipes PR, once you create it. I was just reading up on the state of arrow vs. rust, and was surprised that datafusion isn't yet in conda-forge. ;-)

xhochy · 2021-04-26T12:12:06Z

python/tests/test_df.py

@@ -0,0 +1,98 @@
+import unittest


Out of curiosity: Why not pytest?

it comes with python, so no need to install other stuff. But no feelings here; we can refactor this whole thing. =)

python/Cargo.toml

jorgecarleitao · 2021-04-29T19:21:46Z

Pushed the license and also hopefully fixed the CI.

Co-authored-by: Andy Grove <andygrove@users.noreply.github.com>

Co-authored-by: Uwe L. Korn <xhochy@users.noreply.github.com>

jorgecarleitao · 2021-04-30T09:48:43Z

Ok, I have now fixed the CI, pushed the license headers, and bumped to latest datafusion.

There was a regression, documented in #226.

Once we fix the regression, this can be released in pypi as 0.2.2 since there were no backward incompatible changes on it 🎉

andygrove · 2021-05-04T01:02:10Z

@alamb @Dandandan Any objection to merging this PR?

Dandandan · 2021-05-04T04:54:16Z

@andygrove please go ahead 🚀

alamb

No objections to merging from me. I skimmed it quickly and all seems good.

I think a significant investment in documentation will be needed for this code, but it seems like a good start to me

wesm · 2021-05-04T14:03:10Z

I hate to be a nuisance, but didn't this need to go through IP clearance?

andygrove · 2021-05-04T14:06:18Z

I hate to be a nuisance, but didn't this need to go through IP clearance?

We can revert if this is the case, but because Jorge was the only contributor (except for one contribution fixing a typo in a README) this didn't seem to be required in this case?

wesm · 2021-05-04T14:08:31Z

Probably best to check with general@incubator to determine the preferred protocol in this situation. I don't want to subject you to unneeded process, but would be good to go by the book

jorgecarleitao · 2021-05-04T14:10:15Z

@wesm thank you. Not a nuisance at all, it is important to have this done correctly.

The rational here:

I hold the copyright over the whole code base, except for a 1 word typo fix on the README. The code was MIT licensed on jorgecarleitao/python-datafusion.

As part of this PR, I pushed a commit that added the license headers to every file in the source code. As copyright holder, I thereby licensed all this code to ASF under the ICA.

This reverts commit 46bde0b.

andygrove · 2021-05-04T14:16:26Z

PR to revert: #257

wesm · 2021-05-04T14:33:46Z

Thanks, I'm not enough of an expert to know what is the correct protocol, a vote may not be needed at all but let's double check

This reverts commit 46bde0b.

alamb · 2021-05-04T14:52:04Z

Yeah, my interpretation was that since @jorgecarleitao authored this code, I was treating this as "just a normal PR" (it happens to have lived somewhere else for a while but from an IP provenance perspective it seemed no different to a normal PR to me).

However, I am not an expert in such matters.

This reverts commit d0af907.

andygrove reviewed Apr 25, 2021

View reviewed changes

python/Cargo.toml Outdated Show resolved Hide resolved

andygrove reviewed Apr 25, 2021

View reviewed changes

python/LICENSE Outdated Show resolved Hide resolved

xhochy reviewed Apr 26, 2021

View reviewed changes

jorgecarleitao and others added 7 commits April 30, 2021 08:23

Added Python project.

ef6992c

Update python/Cargo.toml

91ab68e

Co-authored-by: Andy Grove <andygrove@users.noreply.github.com>

Update python/Cargo.toml

67f3279

Co-authored-by: Uwe L. Korn <xhochy@users.noreply.github.com>

Added license and black formatting.

1ae4059

License

d85446e

Fixing build.

86186ea

TesTestt

d471335

jorgecarleitao force-pushed the python branch from f98fe0d to d471335 Compare April 30, 2021 08:23

Bumped to latest DataFusion.

8351836

jorgecarleitao added 3 commits April 30, 2021 09:51

Bumped nightly.

9c82552

Bumped pyarrow in tests.

ab17019

Added some tests back.

3d9e1a3

andygrove approved these changes May 1, 2021

View reviewed changes

alamb approved these changes May 4, 2021

View reviewed changes

andygrove merged commit 46bde0b into apache:master May 4, 2021

andygrove referenced this pull request in andygrove/datafusion May 4, 2021

Revert "Add datafusion-python (#69)"

543639f

This reverts commit 46bde0b.

jorgecarleitao deleted the python branch May 4, 2021 14:10

nevi-me pushed a commit that referenced this pull request May 4, 2021

Revert "Add datafusion-python (#69)" (#257)

d0af907

This reverts commit 46bde0b.

andygrove referenced this pull request in andygrove/datafusion May 5, 2021

Revert "Revert "Add datafusion-python (#69)" (#257)"

39b1c3a

This reverts commit d0af907.

andygrove added a commit that referenced this pull request May 5, 2021

Revert "Revert "Add datafusion-python (#69)" (#257)" (#270)

3be087a

This reverts commit d0af907.

houqp added the python label Jul 29, 2021

andygrove mentioned this pull request Jul 15, 2022

Should we move this library back to Apache Arrow governance? datafusion-contrib/datafusion-python#58

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add datafusion-python #69

Add datafusion-python #69

jorgecarleitao commented Apr 25, 2021

codecov-commenter commented Apr 25, 2021 •

edited

andygrove commented Apr 25, 2021

jorgecarleitao commented Apr 25, 2021

xhochy Apr 26, 2021

jorgecarleitao Apr 26, 2021

xhochy Apr 26, 2021

h-vetinari May 1, 2021

xhochy Apr 26, 2021

jorgecarleitao Apr 26, 2021

jorgecarleitao commented Apr 29, 2021

jorgecarleitao commented Apr 30, 2021

andygrove commented May 4, 2021

Dandandan commented May 4, 2021

alamb left a comment

wesm commented May 4, 2021

andygrove commented May 4, 2021

wesm commented May 4, 2021

jorgecarleitao commented May 4, 2021

andygrove commented May 4, 2021

wesm commented May 4, 2021

alamb commented May 4, 2021

Add datafusion-python #69

Add datafusion-python #69

Conversation

jorgecarleitao commented Apr 25, 2021

codecov-commenter commented Apr 25, 2021 • edited

Codecov Report

andygrove commented Apr 25, 2021

jorgecarleitao commented Apr 25, 2021

xhochy Apr 26, 2021

Choose a reason for hiding this comment

jorgecarleitao Apr 26, 2021

Choose a reason for hiding this comment

xhochy Apr 26, 2021

Choose a reason for hiding this comment

h-vetinari May 1, 2021

Choose a reason for hiding this comment

xhochy Apr 26, 2021

Choose a reason for hiding this comment

jorgecarleitao Apr 26, 2021

Choose a reason for hiding this comment

jorgecarleitao commented Apr 29, 2021

jorgecarleitao commented Apr 30, 2021

andygrove commented May 4, 2021

Dandandan commented May 4, 2021

alamb left a comment

Choose a reason for hiding this comment

wesm commented May 4, 2021

andygrove commented May 4, 2021

wesm commented May 4, 2021

jorgecarleitao commented May 4, 2021

andygrove commented May 4, 2021

wesm commented May 4, 2021

alamb commented May 4, 2021

codecov-commenter commented Apr 25, 2021 •

edited