-
Notifications
You must be signed in to change notification settings - Fork 48
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
[C++] Adding recipe for custom compute functions #227
base: main
Are you sure you want to change the base?
Conversation
In order to get these recipes to appear on https://arrow.apache.org/cookbook/cpp/index.html, we should add a |
Thanks for adding this, I'll take a look. Side note: we should probably just set up clang-format for C++ examples… |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a great example. I left some comments, but as noted we also need an reST file to get this to appear in the Cookbook itself. Of course, this probably needs to wait on 9.0.0 too
thanks for the comments! I will get around to the reST file today. I was putting it off a bit but today feels like a good day for it. |
This recipe shows the major portions of a custom, or new, compute function: - defining a compute kernel - creating a function instance - associating the kernel with the function - registering the function in a registry - calling the function
Mostly minor changes. Only major change is replacing the use of hashing32 from key_hash.c with ScalarHelper from hashing.h
782c639
to
a0bde0a
Compare
rebased to try and build the cookbooks, but I am not sure how to build the documentation for the C++ code |
compute.rst is documentation to describe compute functions. For now it describes how to define and register a compute function. Still a work in progress. Updates to compute_fn.cc are to reflect the description provided in compute.rst
I am not sure how to add whole function definitions to the recipe instead of portions of the body. For example: arrow-cookbook/cpp/code/compute_fn.cc Lines 88 to 90 in 411d471
|
Also added namespace and :class: and :func: annotations. I think it doesn't matter in the cookbook, but it links nicely when in the arrow docs themselves. I'm kind of curious how it renders in the cookbook
Trying to see if moving these recipe labels captures C++ code blocks outside of function bodies
I am trying the literalinclude directive first before trying to use recipe directives in comments
@wjones127 or @lidavidm if either of you have time, could you check this out? I wasn't able to locally confirm that code excerpt directives worked correctly, but I am hoping to reference this cookbook from documentation for authoring compute kernels |
I kicked off CI, but not sure if I'll have time to look in more detail right now |
I am having trouble setting up a build to figure out why it couldn't find the recipe, so I'm just going to shelve it for a bit. |
This recipe shows the major portions of a custom, or new, compute
function:
The aliases are to keep the code as readable as possible, and also to frontload the various dependencies to make it obvious to the reader.
The kernel implementation here is essentially the same as the new FastHash32 function in progress of being added. The reason for this is that there are examples of much simpler functions (AbsoluteValue), and this recipe also helps the reader understand how to call other functions and how to structure non-trivial returns.