Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ValueError when running to_csv in append mode with single_file as True #10441

Merged
merged 8 commits into from Aug 15, 2023
Merged

Conversation

benrutter
Copy link
Contributor

I think this is pretty well described here: #10414

Currently dask adds on "a" to the mode when single_file is True, but this clashes when the mode is already set to "a".

This PR adds in a conditional check the the mode ending with "a" before appending "a" to the end.

Also, testing added to cover the bug fix.

@GPUtester
Copy link
Collaborator

Can one of the admins verify this patch?

Admins can comment ok to test to allow this one PR to run or add to allowlist to allow all future PRs from the same author to run.

@ncclementi
Copy link
Member

add to allowlist

dask/dataframe/io/csv.py Outdated Show resolved Hide resolved
dask/dataframe/io/csv.py Show resolved Hide resolved
Co-authored-by: Hendrik Makait <hendrik@makait.com>
@hendrikmakait hendrikmakait self-requested a review August 14, 2023 15:15
Copy link
Member

@hendrikmakait hendrikmakait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me. I have some additional recommendations for improving the test.

dask/dataframe/io/tests/test_csv.py Show resolved Hide resolved
dask/dataframe/io/tests/test_csv.py Outdated Show resolved Hide resolved
dask/dataframe/io/tests/test_csv.py Outdated Show resolved Hide resolved
benrutter and others added 3 commits August 14, 2023 16:56
Co-authored-by: Hendrik Makait <hendrik@makait.com>
Co-authored-by: Hendrik Makait <hendrik@makait.com>
Copy link
Member

@hendrikmakait hendrikmakait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated test looks great. Thanks for your contribution, @benrutter!

dask/dataframe/io/tests/test_csv.py Outdated Show resolved Hide resolved
dask/dataframe/io/tests/test_csv.py Outdated Show resolved Hide resolved
@hendrikmakait
Copy link
Member

I've committed two minor nit-picks and will merge once CI is done.

@benrutter
Copy link
Contributor Author

I think the failing test here isn't related to this PR? (it's from dask/dataframe/io/tests/test_parquet.py)

@hendrikmakait hendrikmakait merged commit df11ddc into dask:main Aug 15, 2023
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ValueError with invalid mode when running to_csv in append mode.
4 participants