Skip to content

Conversation

reidy-p
Copy link
Contributor

@reidy-p reidy-p commented Jan 13, 2018

@pep8speaks
Copy link

pep8speaks commented Jan 13, 2018

Hello @reidy-p! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on January 14, 2018 at 23:47 Hours UTC

@codecov
Copy link

codecov bot commented Jan 13, 2018

Codecov Report

Merging #19216 into master will decrease coverage by 0.03%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #19216      +/-   ##
==========================================
- Coverage   91.55%   91.52%   -0.04%     
==========================================
  Files         147      147              
  Lines       48812    48827      +15     
==========================================
- Hits        44690    44689       -1     
- Misses       4122     4138      +16
Flag Coverage Δ
#multiple 89.89% <86.66%> (-0.03%) ⬇️
#single 41.69% <6.66%> (-0.02%) ⬇️
Impacted Files Coverage Δ
pandas/core/series.py 94.61% <ø> (ø) ⬆️
pandas/util/testing.py 84.24% <86.66%> (-0.18%) ⬇️
pandas/plotting/_converter.py 65.22% <0%> (-1.74%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 787ab55...33a90dc. Read the comment docs.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

see comments

@jreback jreback added Docs Output-Formatting __repr__ of pandas objects, to_string labels Jan 13, 2018
@reidy-p
Copy link
Contributor Author

reidy-p commented Jan 13, 2018

@jreback did you post comments? I can't see any.

@jreback
Copy link
Contributor

jreback commented Jan 13, 2018

sorry, see #19226 want to parametrize these tests

@reidy-p reidy-p force-pushed the series_to_csv_compress branch from cf32368 to d8a8753 Compare January 13, 2018 23:38
('gzip', gzip.open),
('bz2', bz2.BZ2File),
(pytest.param('xz', lzma.open, marks=td.skip_if_no_lzma)),
def decompress_file(self, src_path, compression):
Copy link
Contributor

Choose a reason for hiding this comment

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

can u move this function to pandas.util.testing as it’s more generally useful for testing

@reidy-p reidy-p force-pushed the series_to_csv_compress branch 2 times, most recently from 84881db to 9313f37 Compare January 14, 2018 19:22
@reidy-p reidy-p force-pushed the series_to_csv_compress branch from 9313f37 to 33a90dc Compare January 14, 2018 23:47
@gfyoung
Copy link
Member

gfyoung commented Jan 15, 2018

@jreback @reidy-p : I like the decompress utility (it should be applied to our DataFrame.to_csv tests), but this patch for Series.to_csv is only a band-aid. We should really implement what you summarized in your comment here to close this issue.

@jreback
Copy link
Contributor

jreback commented Jan 15, 2018

@gfyoung I agree, but it puts this in a central location at least for now.

@jreback jreback added this to the 0.23.0 milestone Jan 15, 2018
@jreback jreback merged commit de42bee into pandas-dev:master Jan 15, 2018
@jreback
Copy link
Contributor

jreback commented Jan 15, 2018

thanks @reidy-p ok would love to work on centralizing the compression tests!

@reidy-p reidy-p deleted the series_to_csv_compress branch January 16, 2018 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Output-Formatting __repr__ of pandas objects, to_string
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pd.Series to_csv method does not recognize "compression"
4 participants