Add Zstandard compression support #43925

jonashaag · 2021-10-08T13:44:52Z

closes #xxxx
tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry

Haven't found any discussion about Zstandard so far but I think it would be a valuable addition to the Pickle compression options.

pep8speaks · 2021-10-08T13:44:56Z

Hello @jonashaag! 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-12-22 09:44:38 UTC

jreback

this is probably ok, but

need to add this to some of the builds
need to update the doc strings for virtually all methods as the routines you changes are for evertyhing (e.g. it should work, unless of course those formats don't support it, e.g. hdf5 likely doesn't).

jonashaag · 2021-10-11T12:39:03Z

Thanks for the review!

Added Zstandard also to CSV writing and other places and updated all the docstrings
Refactored some tests to use _compression_to_extension dictionary
Fixed binary mode detection in io.common for Zstandard.

jreback

the way you are importing / skipping is not inline with other testing.

ci/deps/actions-38-slow.yaml

doc/source/user_guide/io.rst

pandas/_testing/_io.py

pandas/conftest.py

pandas/io/common.py

pandas/io/pickle.py

pandas/tests/io/test_common.py

pandas/tests/io/test_pickle.py

jreback · 2021-10-21T13:03:11Z

cc @twoertwein if you can have a look

pandas/io/common.py

pandas/io/pickle.py

pandas/io/common.py

jreback

ok if you can merge master and address the existing comments

twoertwein

Thank you, looks good from my side!

I think the only pending parts are 1) whatsnew entry for 1.4, 2) pytest.importorskip, 3) not testing zstd on all CI configurations, and 4) rebasing

jonashaag · 2021-12-07T17:43:08Z

Will push an update soon

jonashaag · 2021-12-08T14:30:10Z

I've consolidated all compression docstrings into two shared fragments

setup.py

pandas/core/describe.py

jonashaag · 2021-12-15T12:31:35Z

If this builds OK it should be ready for re-review

jonashaag · 2021-12-15T13:43:27Z

Test failures should go away when this PR is merged (missing .zst URL in this GitHub repo)

twoertwein · 2021-12-15T15:30:16Z

You will need to add "zstd" here

pandas/pandas/_typing.py

Line 246 in 85c221a

Union[Literal["infer", "gzip", "bz2", "zip", "xz"], CompressionDict]

to make mypy happy.

doc/source/user_guide/io.rst

twoertwein · 2021-12-15T15:39:53Z

The only obvious part missing to me is a whatsnew entry.

doc/source/whatsnew/v1.4.0.rst

jreback

looks really good! need to change the shared docs from generic to shared_docs and the circular import will go away.

some small other comments. pls merge master and ping on green.

pandas/core/describe.py

pandas/io/common.py

pandas/io/formats/xml.py

pandas/tests/io/test_pickle.py

twoertwein · 2021-12-19T14:23:31Z

pandas/io/common.py

@@ -1136,18 +1130,20 @@ def _is_binary_mode(handle: FilePath | BaseBuffer, mode: str) -> bool:
    )


-def _get_binary_io_classes() -> tuple[type, ...]:
+def _get_binary_io_classes(_cache: list[type] = []) -> tuple[type, ...]:


How about?

@functools.cache def _get_binary_io_classes() -> tuple[type, ...]:

Python 3.9+ only

@functools.lru_cache should work as well, or?

Sure if you prefer that over the custom memoization I'm happy to change it

There is only one "least recently used" input argument (might be a bit too much to use lru_cache) but it keeps the code slightly simpler. Will defer to @jreback

twoertwein · 2021-12-19T14:27:20Z

pandas/tests/io/conftest.py

+    (".xz", "xz"),
+    (".XZ", "xz"),
+    pytest.param((".zst", "zstd"), marks=td.skip_if_no("zstandard")),
+    pytest.param((".ZST", "zstd"), marks=td.skip_if_no("zstandard")),


If the compression tests take too much time: I would expect that we need only one upper-case test, we have one .lower() call that affects all compression formats. @jreback

Small benchmark: io/test_common.py + io/test_pickle.py with full list: 4.25 s, with small list: 4.2 s

Thank you for the quick benchmark! Based on that we can keep all the upper cases. Compression is also used in a lot of CSV tests.

jonashaag · 2021-12-19T16:17:24Z

@jreback ping, should be good to go now. Test failures are because one of the test only works when this has been merged to master.

jreback

there still seems to be a few uses of generic._share_docs, can you change these to directly use the shared_docs itself (we must alias this but its not great)

pandas/core/frame.py

pandas/io/common.py

pandas/io/json/_json.py

pandas/io/pickle.py

jonashaag · 2021-12-20T09:44:55Z

Something with CI is really broken, all the PRs are red

jonashaag · 2021-12-21T21:55:24Z

@jreback should be good to go

jreback · 2021-12-22T02:29:06Z

can you merge master once more, been having a number of CI issues

jreback · 2021-12-22T15:17:52Z

https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=71120&view=logs&j=9dd5e044-6afa-5be6-cb56-860f1ee82b31&t=d3ae057b-d0af-59ce-6e4c-b69ef3739701 are failing related to zstd

jonashaag · 2021-12-22T19:01:28Z

@jreback looks like those are only failing because those files are not yet on master

jreback · 2021-12-22T22:47:53Z

failing because those files are not yet on master

what files?

jonashaag · 2021-12-22T22:55:14Z

what files?

2021-12-22T10:21:51.1829969Z _______________ test_compressed_urls[python-explicit-zstd-.zst] _______________
...
2021-12-22T10:21:51.1885656Z E       urllib.error.HTTPError: HTTP Error 404: Not Found
2021-12-22T10:21:51.1885903Z 
2021-12-22T10:21:51.1886527Z C:\Miniconda\envs\pandas-dev\lib\urllib\request.py:649: HTTPError
2021-12-22T10:21:51.1887052Z ________________ test_compressed_urls[python-infer-zstd-.zst] _________________

https://github.com/pandas-dev/pandas/tree/master/pandas/tests/io/parser/data

pandas/pandas/tests/io/parser/test_network.py

Lines 38 to 43 in 5f36af3

    
           base_url = ( 
        
               "https://github.com/pandas-dev/pandas/raw/master/" 
        
               "pandas/tests/io/parser/data/salaries.csv" 
        
           ) 
        
           url = base_url + extension

Hardcoded URL access to master branch

jreback · 2021-12-22T22:56:52Z

thanks @jonashaag very nice!

mroeschke · 2021-12-23T00:00:57Z

Looks like master is failing some builds are missing zstandard (or not importskiped)?

https://github.com/pandas-dev/pandas/runs/4612486852?check_suite_focus=true#step:10:255

phofl · 2021-12-23T09:59:45Z

Yep

FAILED pandas/tests/io/parser/test_network.py::test_compressed_urls[python-explicit-zstd-.zst]
FAILED pandas/tests/io/parser/test_network.py::test_compressed_urls[python-infer-zstd-.zst]
FAILED pandas/tests/io/parser/test_network.py::test_compressed_urls[c-explicit-zstd-.zst]
FAILED pandas/tests/io/parser/test_network.py::test_compressed_urls[c-infer-zstd-.zst]

jonashaag force-pushed the pickle-zstd branch from 895845b to ba223c8 Compare October 8, 2021 13:50

jreback requested changes Oct 10, 2021

View reviewed changes

jreback added the IO Data IO issues that don't fit into a more specific label label Oct 10, 2021

jonashaag changed the title ~~Add Zstandard support to read_pickle/to_pickle~~ Add Zstandard compression support Oct 12, 2021

jonashaag requested a review from jreback October 12, 2021 16:44

jreback requested changes Oct 21, 2021

View reviewed changes

twoertwein reviewed Oct 21, 2021

View reviewed changes

pandas/io/common.py Show resolved Hide resolved

pandas/io/pickle.py Outdated Show resolved Hide resolved

jreback requested changes Nov 14, 2021

View reviewed changes

pandas/io/common.py Outdated Show resolved Hide resolved

jreback requested changes Nov 28, 2021

View reviewed changes

twoertwein approved these changes Dec 7, 2021

View reviewed changes

jonashaag force-pushed the pickle-zstd branch from a831405 to 2334af8 Compare December 8, 2021 14:29

jonashaag force-pushed the pickle-zstd branch 2 times, most recently from 850be36 to 532dd8f Compare December 8, 2021 14:36

jonashaag commented Dec 8, 2021

View reviewed changes

setup.py Outdated Show resolved Hide resolved

jonashaag force-pushed the pickle-zstd branch 2 times, most recently from 77c9022 to 760ba0a Compare December 8, 2021 14:43

twoertwein reviewed Dec 8, 2021

View reviewed changes

pandas/core/describe.py Outdated Show resolved Hide resolved

twoertwein reviewed Dec 15, 2021

View reviewed changes

doc/source/user_guide/io.rst Outdated Show resolved Hide resolved

jonashaag force-pushed the pickle-zstd branch from 92ed77e to 6843884 Compare December 15, 2021 15:45

twoertwein reviewed Dec 16, 2021

View reviewed changes

doc/source/whatsnew/v1.4.0.rst Show resolved Hide resolved

twoertwein mentioned this pull request Dec 16, 2021

ENH: add support for reading .tar archives #44787

Merged

3 tasks

jonashaag requested a review from jreback December 16, 2021 12:37

jreback requested changes Dec 18, 2021

View reviewed changes

pandas/core/describe.py Outdated Show resolved Hide resolved

pandas/io/common.py Outdated Show resolved Hide resolved

pandas/io/common.py Show resolved Hide resolved

pandas/io/formats/xml.py Outdated Show resolved Hide resolved

pandas/tests/io/test_pickle.py Outdated Show resolved Hide resolved

twoertwein reviewed Dec 19, 2021

View reviewed changes

jonashaag force-pushed the pickle-zstd branch from 4f6772c to 84d61d0 Compare December 19, 2021 18:11

jreback requested changes Dec 19, 2021

View reviewed changes

pandas/core/frame.py Show resolved Hide resolved

pandas/io/common.py Show resolved Hide resolved

pandas/io/json/_json.py Outdated Show resolved Hide resolved

pandas/io/pickle.py Outdated Show resolved Hide resolved

jreback added this to the 1.4 milestone Dec 19, 2021

jonashaag force-pushed the pickle-zstd branch from 84d61d0 to a3a6c1a Compare December 20, 2021 09:28

jonashaag force-pushed the pickle-zstd branch 2 times, most recently from f331616 to 8bd477f Compare December 21, 2021 20:13

Add Zstandard compression support

735b1db

jonashaag force-pushed the pickle-zstd branch from 8bd477f to 735b1db Compare December 22, 2021 09:44

jreback approved these changes Dec 22, 2021

View reviewed changes

jreback merged commit e50d077 into pandas-dev:master Dec 22, 2021

phofl mentioned this pull request Dec 23, 2021

CI: Add zstandard to env file #45025

Closed

1 task

jonashaag mentioned this pull request Dec 23, 2021

Fix Zstandard compression unit test #45026

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Zstandard compression support #43925

Add Zstandard compression support #43925

jonashaag commented Oct 8, 2021

pep8speaks commented Oct 8, 2021 •

edited

Loading

jreback left a comment

jonashaag commented Oct 11, 2021 •

edited

Loading

jreback left a comment

jreback commented Oct 21, 2021

jreback left a comment

twoertwein left a comment

jonashaag commented Dec 7, 2021

jonashaag commented Dec 8, 2021

jonashaag commented Dec 15, 2021 •

edited

Loading

jonashaag commented Dec 15, 2021

twoertwein commented Dec 15, 2021

twoertwein commented Dec 15, 2021

jreback left a comment

twoertwein Dec 19, 2021

jonashaag Dec 19, 2021

twoertwein Dec 19, 2021

jonashaag Dec 19, 2021

twoertwein Dec 19, 2021

twoertwein Dec 19, 2021

jonashaag Dec 19, 2021

twoertwein Dec 19, 2021

jonashaag commented Dec 19, 2021

jreback left a comment

jonashaag commented Dec 20, 2021

jonashaag commented Dec 21, 2021

jreback commented Dec 22, 2021

jreback commented Dec 22, 2021

jonashaag commented Dec 22, 2021

jreback commented Dec 22, 2021

jonashaag commented Dec 22, 2021 •

edited

Loading

jreback commented Dec 22, 2021

mroeschke commented Dec 23, 2021

phofl commented Dec 23, 2021

Add Zstandard compression support #43925

Add Zstandard compression support #43925

Conversation

jonashaag commented Oct 8, 2021

pep8speaks commented Oct 8, 2021 • edited Loading

Comment last updated at 2021-12-22 09:44:38 UTC

jreback left a comment

Choose a reason for hiding this comment

jonashaag commented Oct 11, 2021 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

jreback commented Oct 21, 2021

jreback left a comment

Choose a reason for hiding this comment

twoertwein left a comment

Choose a reason for hiding this comment

jonashaag commented Dec 7, 2021

jonashaag commented Dec 8, 2021

jonashaag commented Dec 15, 2021 • edited Loading

jonashaag commented Dec 15, 2021

twoertwein commented Dec 15, 2021

twoertwein commented Dec 15, 2021

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonashaag commented Dec 19, 2021

jreback left a comment

Choose a reason for hiding this comment

jonashaag commented Dec 20, 2021

jonashaag commented Dec 21, 2021

jreback commented Dec 22, 2021

jreback commented Dec 22, 2021

jonashaag commented Dec 22, 2021

jreback commented Dec 22, 2021

jonashaag commented Dec 22, 2021 • edited Loading

jreback commented Dec 22, 2021

mroeschke commented Dec 23, 2021

phofl commented Dec 23, 2021

pep8speaks commented Oct 8, 2021 •

edited

Loading

jonashaag commented Oct 11, 2021 •

edited

Loading

jonashaag commented Dec 15, 2021 •

edited

Loading

jonashaag commented Dec 22, 2021 •

edited

Loading