@Li, yes though in a new way. This came up in a data-source UDF scenario where the implementation is a Python stream factory, i.e., a Python callable returning a callable. I think it would be easier to get the factory and then the stream using Python code than using Cython or C++.
Yaron. ________________________________ From: Li Jin <ice.xell...@gmail.com> Sent: Tuesday, July 5, 2022, 4:22 PM To: dev@arrow.apache.org <dev@arrow.apache.org> Subject: Re: accessing Substrait protobuf Python classes from PyArrow Yaron, do we need to parse the subtrait protobuf in Python so that we can get the UDFs and register them with Pyarrow? On Mon, Jul 4, 2022 at 1:24 PM Yaron Gvili <rt...@hotmail.com> wrote: > This rewriting of the package is basically what I had in mind; the `_ep` > was just to signal a private package, which cannot be enforced, of course. > Assuming this rewriting would indeed avoid conflict with any standard > protobuf package a user might want to use, it would be nicer to do this > using a robust refactoring > tool. We could try Rope [1] (in particular, its folder refactoring library > method [2]) for this; it should add Rope only as a build dependency. Does > this sound worth trying? > > [1] https://github.com/python-rope/rope > [2] > https://rope.readthedocs.io/en/latest/library.html#list-of-refactorings > > > Yaron. > ________________________________ > From: Jeroen van Straten <jeroen.van.stra...@gmail.com> > Sent: Monday, July 4, 2022 12:38 PM > To: dev@arrow.apache.org <dev@arrow.apache.org> > Subject: Re: accessing Substrait protobuf Python classes from PyArrow > > Ah, I see. I guess that complicates things a little. > > About the exposure part in C++, the idea is that if we statically link > libprotobuf and use private linkage for the generated classes, *hopefully* > users won't run into linking issues when they link to both Arrow and to > some libprotobuf of their own, especially when they or another one of their > dependencies also includes Substrait protobuf. The classes would absolutely > conflict otherwise, and the libprotobuf version may or may not conflict. > The latter is kind of an unsolved problem; AFAIK there have been talks to > replace libprotobuf with a third-party alternative to avoid all this > nonsense, but nothing concrete yet. > > For Python, the problems are not quite the same: > > - There is no such thing as private linkage in Python, so we *have* to > re-namespace the generated Python files. Note that the output of protoc > assumes that the files are in exactly the same namespace/module > path/whatever as the proto files specify, so the toplevel module would be > "substrait", not "arrow". See > https://github.com/substrait-io/substrait/pull/207 and > > https://github.com/substrait-io/substrait-validator/blob/941b81ddf65d8ee6266d6eea0d151a3f9d8a512b/py/build.rs#L105-L142 > for my ugly workarounds for this thus far. > - There is also no such thing as static linkage in Python, so if you'd > want to use Google's protobuf implementation in Python pyarrow *will* need > to depend on it, and it'd become the user's problem to make sure that the > installed Python protobuf version matches the version of their system > library. It looks like pyarrow currently only depends on numpy, which is > pretty awesome... so I feel like we should keep it that way. > > Not sure what the best course of action is. > > Jeroen > > On Sun, 3 Jul 2022 at 22:55, Yaron Gvili <rt...@hotmail.com> wrote: > > > Thanks, the Google protobuf exposure concerns are clear. Another concern > > would be consistent maintenance of the same version of Substrait protobuf > > used in Arrow C++ and in PyArrow. > > > > I didn't mean access to users but internally. That is, I didn't mean to > > expose Substrait protobuf Python classes to PyArrow users, just to use > them > > internally in PyArrow code I'll be developing, per the use case I > described > > at the start of this thread. IIUC, this should address the exposure > > concerns, and the `arrow/engine/substrait` module in Arrow C++ does this > > too. If the technical approach I just described would actually expose the > > classes, what would be a proper way to avoid exposing them? Perhaps the > > classes should be generated into a private package, e.g., under > > `python/_ep`? (ep stands for external project) > > > > > > Yaron. > > ________________________________ > > From: Antoine Pitrou <anto...@python.org> > > Sent: Sunday, July 3, 2022 3:20 PM > > To: dev@arrow.apache.org <dev@arrow.apache.org> > > Subject: Re: accessing Substrait protobuf Python classes from PyArrow > > > > > > I agree that giving direct access to protobuf classes is not Arrow's > > job. You can probably take the upstream (i.e. Substrait's) protobuf > > definitions and compile them yourself, using whatever settings required > > by your project. > > > > Regards > > > > Antoine. > > > > > > Le 03/07/2022 à 21:16, Jeroen van Straten a écrit : > > > It's not so much about whether or not we *can*, but about whether or > not > > we > > > *should* generate and expose these files. > > > > > > Fundamentally, Substrait aims to be a means to connect different > systems > > > together by standardizing some interchange format. Currently that > happens > > > to be based on protobuf, so *one* (obvious) way to generate and > interpret > > > Substrait plans is to use Google's own protobuf implementation, as > > > generated by protoc for various languages. It's true that there's > nothing > > > fundamentally blocking Arrow from exposing those. > > > > > > However, it's by no means the *only* way to interact with protobuf > > > serializations, and Google's own implementation is a hot mess when it > > comes > > > to dependencies; there are lots of good reasons why you might not want > to > > > depend on it and opt for a third-party implementation instead. For one > > > thing, the Python wheel depends on system libraries beyond manylinux, > and > > > if they happen to be incompatible (which is likely) it just explodes > > unless > > > you set some environment variable. Therefore, assuming pyarrow doesn't > > > already depend on protobuf, I feel like we should keep it that way, and > > > thus that we should not include the generated Python files in the > > release. > > > Note that we don't even expose/leak the protoc-generated C++ classes > for > > > similar reasons. > > > > > > Also, as Weston already pointed out, it's not really our job; Substrait > > > aims to publish bindings for various languages by itself. It just > doesn't > > > expose them for Python *yet*. In the interim I suppose you could use > the > > > substrait-validator package from PyPI. It does expose them, as well as > > some > > > convenient conversion functions, but I'm having trouble finding people > to > > > help me keep the validator maintained. > > > > > > I suppose versioning would be difficult either way, since Substrait > > > semi-regularly pushes breaking changes and Arrow currently lags behind > by > > > several months (though I have a PR open for Substrait 0.6). I guess > from > > > that point of view distributing the right version along with pyarrow > > seems > > > nice, but the issues of Google's protobuf implementation remain. This > > being > > > an issue at all is also very much a Substrait problem, not an Arrow > > > problem; at best we can try to mitigate it. > > > > > > Jeroen > > > > > > On Sun, Jul 3, 2022, 17:51 Yaron Gvili <rt...@hotmail.com> wrote: > > > > > >> I looked into the Arrow build system some more. It is possible to get > > the > > >> Python classes generated by adding "--python-out" flag (set to a > > directory > > >> created for it) to the `${ARROW_PROTOBUF_PROTOC}` command under > > >> `macro(build_substrait)` in > > `cpp/cmake_modules/ThirdpartyToolchain.cmake`. > > >> However, this makes them available only in the Arrow C++ build whereas > > for > > >> the current purpose they need to be available in the PyArrow build. > The > > >> PyArrow build calls `cmake` on `python/CMakeLists.txt`, which AFAICS > has > > >> access to `cpp/cmake_modules`. So, one solution could be to pull > > >> `macro(build_substrait)` into `python/CMakeLists.txt` and call it to > > >> generate the Python protobuf classes under `python/`, making them > > available > > >> for import by PyArrow code. This would probably be cleaner with some > > macro > > >> parameters to distinguish between C++ and Python generation. > > >> > > >> Does this sound like a reasonable approach? > > >> > > >> > > >> Yaron. > > >> > > >> ________________________________ > > >> From: Yaron Gvili <rt...@hotmail.com> > > >> Sent: Saturday, July 2, 2022 8:55 AM > > >> To: dev@arrow.apache.org <dev@arrow.apache.org>; Phillip Cloud < > > >> cpcl...@gmail.com> > > >> Subject: Re: accessing Substrait protobuf Python classes from PyArrow > > >> > > >> I'm somewhat confused by this answer because I think resolving the > > issue I > > >> raised does not require any change outside PyArrow. I'll try to > explain > > the > > >> issue differently. > > >> > > >> First, let me describe the current situation with Substrait protobuf > in > > >> Arrow C++. The Arrow C++ build system handles external projects in > > >> `cpp/cmake_modules/ThirdpartyToolchain.cmake`, and one of these > external > > >> projects is "substrait". By default, the build system takes the source > > code > > >> for "substrait" from ` > > >> > > > https://github.com/substrait-io/substrait/archive/${ARROW_SUBSTRAIT_BUILD_VERSION}.tar.gz > > >> ` where `ARROW_SUBSTRAIT_BUILD_VERSION` is set in > > >> `cpp/thirdparty/versions.txt`. The source code is check-summed and > > unpacked > > >> in `substrait_ep-prefix` under the build directory and from this the > > >> protobuf C++ classes are generated in `*.pb.{h,cc}` files in > > >> `substrait_ep-generated` under the build directory. The build system > > makes > > >> a library using the `*.cc` files and makes the `*.h` files available > for > > >> other C++ modules to use. > > >> > > >> Setting up the above mechanism did not require any change in the > > >> `substrait-io/substrait` repo, nor any coordination with its authors. > > What > > >> I'm looking for is a similar build mechanism for PyArrow that builds > > >> Substrait protobuf Python classes and makes them available for use by > > other > > >> PyArrow modules. I believe this PyArrow build mechanism does not exist > > >> currently and that setting up one would not require any changes > outside > > >> PyArrow. I'm asking (1) whether that's indeed the case, (2) whether > > others > > >> agree this mechanism is needed at least due to the problem I ran into > > that > > >> I previously described, and (3) for any thoughts about how to set up > > this > > >> mechanism assuming it is needed. > > >> > > >> Weston, perhaps your thinking was that the Substrait protobuf Python > > >> classes need to be built by a repo in the substrait-io space and made > > >> available as a binary+headers package? This can be done but will > require > > >> involving Substrait people and appears to be inconsistent with current > > >> patterns in the Arrow build system. Note that for my purposes here, > the > > >> Substrait protobuf Python classes will be used for composing or > > >> interpreting a Substrait plan, not for transforming it by an > optimizer, > > >> though a Python-based optimizer is a valid use case for them. > > >> > > >> > > >> Yaron. > > >> ________________________________ > > >> From: Weston Pace <weston.p...@gmail.com> > > >> Sent: Friday, July 1, 2022 12:42 PM > > >> To: dev@arrow.apache.org <dev@arrow.apache.org>; Phillip Cloud < > > >> cpcl...@gmail.com> > > >> Subject: Re: accessing Substrait protobuf Python classes from PyArrow > > >> > > >> Given that Acero does not do any planner / optimizer type tasks I'm > > >> not sure you will find anything like this in arrow-cpp or pyarrow. > > >> What you are describing I sometimes refer to as "plan slicing and > > >> dicing". I have wondered if we will someday need this in Acero but I > > >> fear it is a slippery slope between "a little bit of plan > > >> manipulation" and "a full blown planner" so I've shied away from it. > > >> My first spot to look would be a substrait-python repository which > > >> would belong here: https://github.com/substrait-io > > >> > > >> However, it does not appear that such a repository exists. If you're > > >> willing to create one then a quick ask on the Substrait Slack instance > > >> should be enough to get the repository created. Perhaps there is some > > >> genesis of this library in Ibis although I think Ibis would use its > > >> own representation for slicing and dicing and only use Substrait for > > >> serialization. > > >> > > >> Once that repository is created pyarrow could probably import it but > > >> unless this plan manipulation makes sense purely from a pyarrow > > >> perspective I would rather prefer that the user application import > > >> both pyarrow and substrait-python independently. > > >> > > >> Perhaps @Phillip Cloud or someone from the Ibis space might have some > > >> ideas on where this might be found. > > >> > > >> -Weston > > >> > > >> On Thu, Jun 30, 2022 at 10:06 AM Yaron Gvili <rt...@hotmail.com> > wrote: > > >>> > > >>> Hi, > > >>> > > >>> Is there support for accessing Substrait protobuf Python classes > (such > > >> as Plan) from PyArrow? If not, how should such support be added? For > > >> example, should the PyArrow build system pull in the Substrait repo as > > an > > >> external project and build its protobuf Python classes, in a manner > > similar > > >> to how Arrow C++ does it? > > >>> > > >>> I'm pondering these questions after running into an issue with code > I'm > > >> writing under PyArrow that parses a Substrait plan represented as a > > >> dictionary. The current (and kind of shaky) parsing operation in this > > code > > >> uses json.dumps() on the dictionary, which results in a string that is > > >> passed to a Cython API that handles it using Arrow C++ code that has > > access > > >> to Substrait protobuf C++ classes. But when the Substrait plan > contains > > a > > >> bytes-type, json.dump() no longer works and fails with "TypeError: > > Object > > >> of type bytes is not JSON serializable". A fix for this, and a better > > way > > >> to parse, is using google.protobuf.json_format.ParseDict() [1] on the > > >> dictionary. However, this invocation requires a second argument, > namely > > a > > >> protobuf message instance to merge with. The class of this message > > (such as > > >> Plan) is a Substrait protobuf Python class, hence the need to access > > such > > >> classes from PyArrow. > > >>> > > >>> [1] > > >> > > > https://googleapis.dev/python/protobuf/latest/google/protobuf/json_format.html > > >>> > > >>> > > >>> Yaron. > > >> > > > > > >
