Fix dask_cudf.read_csv
#17612
Fix dask_cudf.read_csv
#17612
Conversation
rjzamora
added
bug
There was a problem hiding this comment.
NOTE: This code was translated from the "legacy" code in dask_cudf/_legacy/io/csv.py. After this PR is merged, that file can be removed in #17558
| # Test chunksize deprecation | ||
| with pytest.warns(FutureWarning, match="deprecated"): | ||
| df3 = dask_cudf.read_csv(path, chunksize=None, dtype=typ) | ||
| dd.assert_eq(df, df3) |
There was a problem hiding this comment.
This was deprecated a long time ago, the new code path doesn't try to catch this anymore.
| with pytest.warns(match="dask_cudf.io.csv.read_csv is now deprecated"): | ||
| df2 = dask_cudf.io.csv.read_csv(csv_path) | ||
| dd.assert_eq(df, df2, check_divisions=False) |
There was a problem hiding this comment.
The new csv code now lives at this "expected" path.
| import dask_expr as dx | ||
| from fsspec.utils import stringify_path | ||
| try: | ||
| # TODO: Remove when cudf is pinned to dask>2024.12.0 |
There was a problem hiding this comment.
Does it make sense for us to do this? rapids-dask-dependency should always pick the latest Dask, no?
There was a problem hiding this comment.
By "when cudf is pinned", I mean "when rapids-dask-dependency is pinned". Is that your question?
rapids-dask-dependency currently pins to >=2024.11.2. This means another package with a dask<2024.12.0 requirement can still give us dask-2024.11.2 in practice, no?
| >>> import dask_cudf | ||
| >>> df = dask_cudf.read_csv("myfiles.*.csv") | ||
|
|
||
| In some cases it can break up large files: |
There was a problem hiding this comment.
What cases are those? Or is it always dependent upon the file size and the value of blocksize? If the latter, maybe we should just rephrase to "It can also break up large files by specifying the size of each block via blocksize".
There was a problem hiding this comment.
Sorry, I didn't spend any time reviewing these doc-strings, because they were directly copied from dask_cudf/_legacy/io/csv.py. Does it make sense to address these suggestions/questions in a follow-up (just to make sure CI is unblocked)?
|
|
||
| >>> df = dask_cudf.read_csv("largefile.csv", blocksize="256 MiB") | ||
|
|
||
| It can read CSV files from external resources (e.g. S3, HTTP, FTP) |
There was a problem hiding this comment.
| It can read CSV files from external resources (e.g. S3, HTTP, FTP) | |
| It can read CSV files from external resources (e.g. S3, HTTP, FTP): |
| ---------- | ||
| path : str, path object, or file-like object | ||
| Either a path to a file (a str, :py:class:`pathlib.Path`, or | ||
| py._path.local.LocalPath), URL (including http, ftp, and S3 |
There was a problem hiding this comment.
| py._path.local.LocalPath), URL (including http, ftp, and S3 | |
| ``py._path.local.LocalPath``), URL (including HTTP, FTP, and S3 |
There was a problem hiding this comment.
Maybe it could also just be :py:class:py._path.local.LocalPath?
| path : str, path object, or file-like object | ||
| Either a path to a file (a str, :py:class:`pathlib.Path`, or | ||
| py._path.local.LocalPath), URL (including http, ftp, and S3 | ||
| locations), or any object with a read() method (such as |
There was a problem hiding this comment.
| locations), or any object with a read() method (such as | |
| locations), or any object with a ``read()`` method (such as |
| The target task partition size. If ``None``, a single block | ||
| is used for each file. | ||
| **kwargs : dict | ||
| Passthrough key-word arguments that are sent to |
There was a problem hiding this comment.
| Passthrough key-word arguments that are sent to | |
| Passthrough keyword arguments that are sent to |
rjzamora
added
5 - Ready to Merge
|
/merge |
Description
Recent changes in dask and dask-expr have broken
dask_cudf.read_csv(dask/dask-expr#1178, dask/dask#11603). Fortunately, the breaking changes help us avoid legacy CSV code in the long run.Checklist