https://issues.apache.org/jira/browse/ARROW-16616

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

I have no idea whether we want to expose such lazy construction APIs on Dataset.

cc @jorisvandenbossche

I have no idea whether we want to expose such lazy construction APIs on Dataset.

FYI, this task has spawned from #13155 (comment) discussion

@pitrou Addressed all comments, should be ready for re-review

I am personally also a bit wary of adding a new public class like FilteredDataset (at least until we have had the broader discussion about how we want to provide a dataframe-like / query object interface, as similar discussions will keep coming up for other methods).
If we want to provide this filter() method on the short term, I would also prefer doing it just on Dataset, as Weston suggested (that was also my original idea for this issue). Although that also creates its backwards compatibility issues of course, if we later let this method return an object backed by a query plan, as that then might not keep all methods/attributes that are currently available on Dataset.

get_fragments is also another method where the user could expect this filter to be applied (giving the same result as specifying the filter keyword)

I'm not completely against this but having FilteredDataset instead of something like Query might be a bit short-sighted. What happens if a user wants to add a dynamic column (project)?

If you had both a projection expression and a filter expression that might be more close to what scanner / datasets provides today.

@westonpace I'm not particularly attached to the FilteredDataset name, I just want to avoid using the Query name explicitly to ensure we avoid hinting users that it's ever supposed to become a fully fledged query system at the moment. They can use IBIS if they are looking for that.

I also dislike the idea of reusing the Scanner class as it smells like hijacking its data read responsibility. I wanted a name that conveys correctly the idea of something that represents a dataset with an applied transformation and to which additional transformations can be applied. Maybe QueriedDataset could do? Smells a lot like the query already happened, so not exactly what I was looking for. I'm open to suggestions. Naming things correctly seems hard, I could try invalidating some caches instead ;)

From #13409 (comment)

If self._filter can be None then what is the advantage of creating a separate FilteredDataset instead of just adding _filter to the existing Dataset?

That was the original implementation, and I was asked to explicitly move it in a dedicated class. Which I think in the end makes sense, better have a single responsibility per class.

Could you expand on this a bit? (I don't know where or why it was asked to move to a dedicated class, the only reference I find in the other PR is the question if this shouldn't live on the Scanner)

It seems to me that if we want to expose a helper filter() method (although it doesn't give that much of value compared to passing the filter to the method that actually will do the scanning, i.e. to_table(..), to_batches(..), etc), adding it just to the main Dataset class will expose the least amount of new API that we "lock in" (it avoids deciding now if we want some "Query" like class)

@jorisvandenbossche I checked for ParquetDataset and the experience is fairly confusing from the end user point of view.

If the dataset is created using ds.parquet_dataset it will have the filter capabilities, but if it's created using pyarrow.parquet.ParquetDataset it won't have filtering capabilities. But ParquetDataset in its V2 implementation is just a proxy to ds.Dataset, so it could in theory gain filtering support.

It seems that ParquetDataset is mostly a duplicate of what ds.parquet_dataset can do when use_legacy_dataset=False, so is there a reason why we keep it around? Is there a plan to deprecate it in the future?

Asking because if the plan is to deprecate it some day, then it probably doesn't make much sense to invest the effort to work toward feature parity with ds.Dataset and we can consider this task done.

I think you certainly don't have to care about ParquetDataset in this PR (we generally didn't add any of the additional methods from pyarrow.dataset.Dataset to it, so this PR just follows that). And let's have the discussion about what to do with ParquetDataset in a dedicated place (eg we already have https://issues.apache.org/jira/browse/ARROW-9720, although the description there is a bit outdated)