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-2537: [Ruby] Import #1990

Closed
wants to merge 1 commit into from
Closed

ARROW-2537: [Ruby] Import #1990

wants to merge 1 commit into from

Conversation

kou
Copy link
Member

@kou kou commented May 3, 2018

elif p in ('c_glib', 'integration', 'python', 'site', 'rust'):
elif p in ('c_glib'):
affected[p] = True
affected['ruby'] = True
Copy link
Member

Choose a reason for hiding this comment

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

The Ruby wrapper binds to the C/Glib binaries?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.
It uses Apache Arrow C++ through Apache Arrow GLib.

@wesm
Copy link
Member

wesm commented May 3, 2018

@kou, this is great, thank you! Since this is a large code import, we will want to go through the IP clearance process like with Go recently http://incubator.apache.org/ip-clearance/. This should not be too painful but will require a vote on the mailing list and then a lazy consensus clearance vote on general@incubator

@kou
Copy link
Member Author

kou commented May 4, 2018

@wesm OK. I'll read the document.

@kou
Copy link
Member Author

kou commented May 10, 2018

@wesm I read the document. I think that I don't have "incubator karma". Can you help me to process this?

  1. IP Clearance processing must be executed either by an Officer or a Member of the ASF. If you are not an Officer or a Member, please contact your project chair who will find an appropriate volunteer. Incubator karma is also required. Please request karma from the incubator pmc if you do not have it.

I filled some fields:

Title:

Arrow Ruby Library Intellectual Property (IP) Clearance Status

Description:

The Arrow Ruby Library is a Ruby language bindings of the Apache Arrow columnar format

Project info:

he Apache Arrow PMC will be responsible for the code.

It will be integrated into the Apache Arrow project, into a new Ruby area of the main Arrow source tree and build system.

The following people will be managing this contribution:

  • TBD
  • ...

Completed tasks are shown by the completion date (YYYY-MM-dd).

Identify the codebase:

Copyright:

Identify name recorded for software grant: Ruby Bindings of Apache Arrow

Verify distribution rights:

Corporations and individuals holding existing distribution rights:

  • Kouhei Sutou

DISCUSS: red-data-tools/red-arrow@701386c is only a commit made by me. It just fix a URL. Is there a copyright of mikisou? If there is a copyright of him, I'll contact to him.

Generally, the result of checking off these items will be a Software Grant, CLA, and Corporate CLA for ASF licensed code, which must have no dependencies upon items whose licenses that are incompatible with the Apache License.

Organizational acceptance of responsibility for the project:

Related votes:

  • TBD

@wesm
Copy link
Member

wesm commented May 11, 2018

hi @kou -- yes, I will help you with the IP clearance. I'm an ASF Member now so I believe I have enough karma to make changes to the IP clearance files.

First, we need to have a PMC vote to accept the Ruby bindings. I'm going to start this now on the mailing list. We can get the IP clearance page set up while that is running, so we can have the Incubator clearance vote next week and then merge the PR.

Sorry for the delay; I have been traveling the last week and not available much on e-mail or GitHub

@kou
Copy link
Member Author

kou commented May 12, 2018

Thanks! I understand the schedule.

Sorry for the delay; I have been traveling the last week and not available much on e-mail or GitHub

No problem. Thanks for your help.

@wesm
Copy link
Member

wesm commented May 24, 2018

The IP clearance vote has passed. I'm going to rebase this and review, and merge on a green build if there are no problems

@codecov-io
Copy link

Codecov Report

Merging #1990 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1990   +/-   ##
=======================================
  Coverage   86.33%   86.33%           
=======================================
  Files         242      242           
  Lines       41103    41103           
=======================================
  Hits        35487    35487           
  Misses       5616     5616

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 5c5e95a...cb41c7a. Read the comment docs.

Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

Some minor comments from my read through. Merging this PR. Thanks @kou!

@@ -0,0 +1,585 @@

Copy link
Member

Choose a reason for hiding this comment

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

Does the Apache license need to be reproduced here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! We should remove it and ruby/red-arrow/LICENSE.txt because we don't have additional license information for Apache Arrow Ruby.


## Install

Install Apache Arrow GPU GLib before install Red Arrow GPU. Use [packages.red-data-tools.org](https://github.com/red-data-tools/packages.red-data-tools.org) for installing Apache Arrow GPU GLib.
Copy link
Member

Choose a reason for hiding this comment

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

It should be noted here that these binaries are not official; I hope we are able to produce official GPU-enabled binaries as part of release votes in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. I'll put the same note in Apache Arrow GLib.

class CUDADeviceManager
# Experimental.
#
# Can we think device manager is a container of contexts?
Copy link
Member

Choose a reason for hiding this comment

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

Yes, most likely -- it would be useful to have some design review of the interface between the CUDA driver API and Apache Arrow. What I did was experimental to help get things started

Copy link
Member Author

Choose a reason for hiding this comment

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

OK!


## Install

Install Apache Arrow GLib before install Red Arrow. Use [packages.red-data-tools.org](https://github.com/red-data-tools/packages.red-data-tools.org) for installing Apache Arrow GLib.
Copy link
Member

Choose a reason for hiding this comment

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

Similar comment as above

module Arrow
# Experimental
#
# TODO: Almost codes should be implemented in Apache Arrow C++.
Copy link
Member

Choose a reason for hiding this comment

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

Definitely intend to build this functionality in Arrow C++ eventually

module Arrow
# Experimental
#
# TODO: Almost codes should be implemented in Apache Arrow C++.
Copy link
Member

Choose a reason for hiding this comment

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

Definitely

# under the License.

module Arrow
# TODO: Almost codes should be implemented in Apache Arrow C++.
Copy link
Member

Choose a reason for hiding this comment

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

I agree

@wesm
Copy link
Member

wesm commented May 25, 2018

+1

@wesm wesm closed this in 338e597 May 25, 2018
@kou kou deleted the ruby-import branch May 25, 2018 01:22
@kou
Copy link
Member Author

kou commented May 25, 2018

Thanks!!!
I'll send some follow-up pull requests to resolve your comments.

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