Skip to content

Conversation

@wesm
Copy link
Member

@wesm wesm commented May 24, 2020

This provides the CAST(data AS target_type) SQL idiom. The target_type is provided via CastOptions (FWIW I believe this is the most correct approach for handling the target_type). As a result we no longer need to maintain separate binding boilerplate in Python for Array vs. ChunkedArray

@github-actions
Copy link

@wesm
Copy link
Member Author

wesm commented May 26, 2020

I had copy-pasted the original docstring for Array.cast. I'll clean it up per above

@wesm
Copy link
Member Author

wesm commented May 26, 2020

Fixed. I also moved the cast Python wrapper to compute.py so it doesn't need to be compiled

@wesm
Copy link
Member Author

wesm commented May 26, 2020

I missed that there was a partial wrapper for CastOptions used in _dataset.pyx, fixing that now

@wesm
Copy link
Member Author

wesm commented May 26, 2020

I just moved the compute wrappers to compute.pxi (so they are part of pyarrow.lib now) and fixed up the usage of CastOptions between pyarrow.compute.cast and _dataset.pyx. So this patch will need to be merged before doing more work on the compute bindings

@wesm
Copy link
Member Author

wesm commented May 27, 2020

Could someone review this?

@jorisvandenbossche
Copy link
Member

Is there a specific reason for moving compute into pyarrow.lib as well? (I would think multiple cython compilation units might be more "scalable" regarding code maintenance etc)

@jorisvandenbossche
Copy link
Member

For the rest looks good to me!

@wesm
Copy link
Member Author

wesm commented May 28, 2020

@jorisvandenbossche I can try moving it back, I was concerned about having to cimport things back into lib.pyx

@jorisvandenbossche
Copy link
Member

What would need to be cimported in lib.pyx? I think in the Array and Table classes etc, for the computational methods, you do the _pc.call_function(..). Or is it for the Option classes? (_dataset.pyx could also cimport that from _compute, like it does for the FileSystem for example)

(now, I don't know how important this is, it was just a throught)

…ager casts, use in Python

Typos

Rebase and fix bugs

Review comments. Move cast to compute.py

Consolidate _compute.pyx, compute.pxi and CastOptions wrapping

decruft

Move compute-related code to _compute.pyx/_compute.pxd
@wesm
Copy link
Member Author

wesm commented May 28, 2020

I added a _compute.pxd, no big deal. Will merge this once the build passes

@wesm
Copy link
Member Author

wesm commented May 28, 2020

Merging. The ARM failure (not sure what went wrong?) does not need to block

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.

2 participants