Remove statistics-based set_index logic from read_parquet #9661
Conversation
|
+1 for the change itself. I don't think we should auto set the index. From a UX perspective, I could see this being a utility function answering the question "Are there sorted columns that could be useful to be an index in my dataset" but setting it automatically without opt-out appears to be strange. This is likely breaking to some. I'm wondering if we want to communicate this somehow. How did we deal with these situations in the past? Code also LGTM. I'm OK with merging after a brief discussion about deprecation cylce |
Good point. Although this PR changes the code to be more consistent with documentation, I agree that some users may be expecting the "undocumented" behavior that we are trying to fix. The good news is that the only case we are really changing here is when |
| def test_select_filtered_column(tmp_path, engine): | ||
|
|
||
| df = pd.DataFrame({"a": range(10), "b": ["cat"] * 10}) | ||
| path = tmp_path / "test_select_filtered_column.parquet" | ||
| df.to_parquet(path, index=False) | ||
|
|
||
| with pytest.warns(UserWarning, match="Sorted columns detected"): | ||
| ddf = dd.read_parquet(path, engine=engine, filters=[("b", "==", "cat")]) | ||
| assert_eq(df, ddf) |
There was a problem hiding this comment.
The fact that you are deleting so much logic and you are not even deleting a test but add a new one is possibly the most compelling reason why this feature needs to go
dd.read_parquetwill only gather statistics for columns that either (1) have been designated as an index or (2) are being filtered. For this reason, it no longer makes sense to auto-infer an index that was not specified in the pandas metadata or by the user (viadd.read_parquet(..., index=)). By using statistics to automatically set an arbitrary sorted column as the index, we open ourselves up to problematic/surprising behavior (see dask-contrib/dask-sql#903 (comment)).This PR proposes that we officially remove the logic use to automatically select an index column using statistics.
pre-commit run --all-files