ENH: Make to_csv('filename.csv.zip') compress the output #40387
Conversation
|
Hello @HQDragon! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-03-16 00:40:58 UTC |
|
CI check failed is not caused by changed files, it exist in master branch. |
| ], | ||
| ) | ||
| def test_to_csv_content_name_in_zipped_file(self, df, csv_name): | ||
| from pathlib import Path |
|
@HQDragon can you add a whatsnew note also the precommit checks are failing |
| # ensure_clean will add random str before suffix_zip_name, | ||
| # need Path.stem to get real file name | ||
| df.to_csv(pth) | ||
| zf = ZipFile(pth) |
There was a problem hiding this comment.
with ZipFile(pth) as zf:
...
|
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
There was a problem hiding this comment.
Thanks @HQDragon, can you merge master and address the comments please? I think this is close to ready.
I guess the CI failure is unrelated, I don't see any docstring change here and that's the failure reason. So, should be fixed when you merge master.
Thanks!
| if archive_name is None and isinstance(file, (os.PathLike, str)): | ||
| archive_name = os.path.basename(file) | ||
| if archive_name.endswith(".zip"): | ||
| self.archive_name = archive_name[:-4] |
There was a problem hiding this comment.
Any reason why to not use pathlib here? This code would look more natural.
| df = DataFrame({"ABC": [1]}) | ||
| with tm.ensure_clean("to_csv_archive_name.zip") as path: | ||
| df.to_csv( | ||
| path, compression={"method": compression, "archive_name": archive_name} | ||
| ) | ||
| with ZipFile(path) as zp: | ||
| expected_arcname = path if archive_name is None else archive_name | ||
| if archive_name is None: |
There was a problem hiding this comment.
why this if and not simply replacing path by Path(path).stem in the original line?
| with ZipFile(pth) as zf: | ||
| result = zf.filelist[0].filename | ||
| expected = pp.stem | ||
| assert result == expected |
There was a problem hiding this comment.
Personal preference would be to simply assert zf.filelist[0].filename == Path(path).stem instead of this incremental approach. I think it's way quicker to read and understand. Also, better to avoid using pp, pth and other cryptic names, descriptive names even if a bit longer make everyone's life easier IMHO.
datapythonista
changed the title
@HQDragon, can you merge master and address the comments please? |
|
Appears that this PR has gone stale thought it's close to ready. Going to close for now but let us know if you'd like to continue working on this and we'd be happy to reopen. |
close old PR,do some work after read pre commit guidelines