[Python] Allow lazy evaluation of filters in Dataset and add Datset.filter method #31969

Activity

1. asfimport commented on Jun 21, 2022

   

   asfimport

   on Jun 21, 2022

   Author

   More actions

   Will Jones / @wjones127:
   I'm a little cautious about the idea of Dataset.filter() returning a Dataset, since it mixes up the purpose of Dataset and Scanner. If it did, what would this return then?

   ds.filter(ds.field("x") > 2).scan(filter=ds.field("y") < 3)

   I think it might be better if Dataset.filter() returned a scanner with that filter. And then Scanner could also have filter() and project() methods to allow chaining. So you could do:

   (
     ds.filter(ds.field("x") > 2)
       .filter(ds.field("y") < 4)
       .project("x", "y", "z")
       .to_table()
   )

   What do you think of that?
2. asfimport commented on Jun 22, 2022

   

   asfimport

   on Jun 22, 2022

   Author

   More actions

   Alessandro Molina / @amol-:
   I'm not fond of relying on Scanner because it looks like violating the single responsibility principle as it would have to grow nearly the same capabilities of Dataset (for example if I want to do a join over filtered data, we would have to implement Scanner.join which doesn't seem to make much sense).

   Another direction I thought about was going toward having a FilteredDataset class, that would still inherit from Dataset but more clearly provide a separation of concerns.
3. asfimport commented on Jun 22, 2022

   

   asfimport

   on Jun 22, 2022

   Author

   More actions

   Neal Richardson / @nealrichardson:
   In the R package we handle this by building a separate query object that contains the Dataset (or RecordBatchReader, or Table, or whatever), and the query object contains the filtering etc. methods. So here, Dataset.filter would first create the query object from the Dataset and then calls the filter method on it.

   That said, going that route (or the FilteredDataset path, for that matter) feels like a path to essentially creating ibis in pyarrow. I'd caution you to think twice before adding APIs like this since they become sticky and hard to remove later (look at how long ParquetDataset has lived past the creation of the general Dataset class).
4. asfimport commented on Jun 22, 2022

   

   asfimport

   on Jun 22, 2022

   Author

   More actions

   Will Jones / @wjones127:

   In the R package we handle this by building a separate query object that contains the Dataset (or RecordBatchReader, or Table, or whatever), and the query object contains the filtering etc. methods.
   Yeah, I think it seems like the PR as-is would be trying to make Dataset into this kind of query object, which I don't think is it's purpose. Scanner seems closer to that to me, but it's also a bit of a mismatch.
   That said, going that route (or the FilteredDataset path, for that matter) feels like a path to essentially creating ibis in pyarrow.
   IMO we need to have a larger conversation about how much of Acero we want to expose in PyArrow. We might want a low-level API for creating exec plans, and maybe even some high-level functions like this. But seems like we should have a design doc to think about what our vision in here and how that does relate to developments in Ibis.
5. asfimport commented on Jun 22, 2022

   

   asfimport

   on Jun 22, 2022

   Author

   More actions

   Neal Richardson / @nealrichardson:
   Scanner is one-shot though, you can't filter to create a Scanner and then filter it again, you can only Scan.
6. asfimport commented on Jun 22, 2022

   

   asfimport

   on Jun 22, 2022

   Author

   More actions

   Will Jones / @wjones127:

   Scanner is one-shot though, you can't filter to create a Scanner and then filter it again, you can only Scan.
   Yeah, then I think we would need some other query class for representing deferred computations with Acero. Which again goes into the question of PyArrow vs Ibis purpose.
7. asfimport commented on Jun 22, 2022

   

   asfimport

   on Jun 22, 2022

   Author

   More actions

   Alessandro Molina / @amol-:
   I don't see much of a conflict with Ibis on long term. Ibis is an interface to multiple query engines and the queries you build there can be run against different targets. It serves the purpose of having a single interface to an infrastructure agnostic environment. Write your Ibis code locally and deploy it against a production system that might run something very different from a Acero in-memory.

   PyArrow instead only exposes the features that are already available in Arrow itself, and it's something all the other bindings are doing too (R and Java do expose access to the compute engine). What you write in pyarrow won't be able to really grow too much in the direction of scaling on multiple nodes, so it's better positioned for quick data discovery without having to involve external dependencies than for the actual final product which will probably want to be based on IBIS.
8. asfimport commented on Jun 27, 2022

   

   asfimport

   on Jun 27, 2022

   Author

   More actions

   Weston Pace / @westonpace:
   I agree that scanner should not be used. The term "scanner", in general, should probably go away from the public API. The scanner in python is more of a record batch reader.

   I'm not sure I have much to add to the "dataset" vs "query" terminology.
9. asfimport commented on Jul 25, 2022

   

   asfimport

   on Jul 25, 2022

   Author

   More actions

   Krisztian Szucs / @kszucs:
   Postponing to 10.0, feel free to include it when the PR is ready.
10. asfimport commented on Dec 12, 2022

    

    asfimport

    on Dec 12, 2022

    Author

    More actions

    Alessandro Molina / @amol-:
    Issue resolved by pull request 13409
    #13409
11. asfimport

    closed this as completedon Dec 12, 2022
12. asfimport

    assigned

    amol-

    on Jan 11, 2023
13. asfimport

    added this to the 11.0.0 milestone on Jan 11, 2023