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.

Sign up for GitHub

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

Jump to bottom

BUG: Infer compression by default in read_fwf() (#22199) #22200

Merged
merged 1 commit into from
Dec 27, 2018
Merged

BUG: Infer compression by default in read_fwf() (#22199) #22200

merged 1 commit into from
Dec 27, 2018

Conversation

NasaGeek
Copy link
Contributor

@NasaGeek NasaGeek commented Aug 5, 2018 •
edited
Loading

jbrockmendel
jbrockmendel requested changes Aug 5, 2018
View reviewed changes
Copy link
Member

@jbrockmendel jbrockmendel left a comment

Choose a reason for hiding this comment

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

Looks fine, needs a test.

@gfyoung gfyoung added the IO Data IO issues that don't fit into a more specific label label Aug 6, 2018
@codecov
Copy link

codecov bot commented Aug 6, 2018 •
edited
Loading

Codecov Report

Merging #22200 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #22200   +/-   ##
=======================================
  Coverage    92.3%    92.3%           
=======================================
  Files         163      163           
  Lines       51943    51943           
=======================================
  Hits        47946    47946           
  Misses       3997     3997
Flag Coverage Δ
#multiple 90.71% <100%> (ø) ⬆️
#single 43% <100%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/io/parsers.py 95.38% <100%> (ø) ⬆️

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 fc7bc3f...6b36015. Read the comment docs.

@jreback
Copy link
Contributor

jreback commented Nov 1, 2018

can you merge master and update

@NasaGeek NasaGeek force-pushed the fwf-infer-compression-default branch from a5898f8 to b0c6c4a Compare November 4, 2018 00:11
@pep8speaks
Copy link

pep8speaks commented Nov 4, 2018 •
edited
Loading

Hello @NasaGeek! Thanks for updating the PR.

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

Comment last updated on December 24, 2018 at 00:18 Hours UTC

@NasaGeek
Copy link
Contributor Author

NasaGeek commented Nov 4, 2018

Rebased and expanded a test

jreback
jreback requested changes Nov 4, 2018
View reviewed changes
@jreback
Copy link
Contributor

jreback commented Nov 11, 2018

can you merge master and update according to fixtures as pointed to by @gfyoung

@jreback
Copy link
Contributor

jreback commented Dec 3, 2018

@NasaGeek can you merge master and update

@jreback
Copy link
Contributor

jreback commented Dec 14, 2018

@NasaGeek can you merge master (or @gfyoung if you can)

@NasaGeek
Copy link
Contributor Author

Happy to rebase, @jreback, but unfortunately I don't yet have an approach to your parametrization request. The available fixture doesn't quite give me enough to work with by itself. It looks like pandas.util.testing.decompress_file may provide a solution if it's augmented to allow return writable file objects. The issue there is that the open function in python2's zipfile module will not give back writable files. Would you be willing to miss coverage on zipfile in python2? If so, what's the best way to skip it?

@gfyoung
Copy link
Member

gfyoung commented Dec 19, 2018

@NasaGeek : You raise some good points regarding the fixtures. I'm going to first do some refactoring on the test_read_fwf.py file itself first before revisiting this PR.

@gfyoung gfyoung mentioned this pull request Dec 19, 2018
Merged
@gfyoung gfyoung force-pushed the fwf-infer-compression-default branch from b0c6c4a to 171a640 Compare December 19, 2018 23:20
@gfyoung gfyoung added Bug Compat pandas objects compatability with Numpy or Python functions labels Dec 19, 2018
@NasaGeek
Copy link
Contributor Author

Just wanted to confirm you didn't need anything else from me on this PR. @gfyoung's changes look fine to me.

jreback
jreback requested changes Dec 23, 2018
View reviewed changes
@gfyoung gfyoung force-pushed the fwf-infer-compression-default branch from 171a640 to 6b36015 Compare December 24, 2018 00:18
@jreback jreback added this to the 0.24.0 milestone Dec 24, 2018
@jreback
Copy link
Contributor

jreback commented Dec 24, 2018

lgtm. ping on green. @jbrockmendel if you have comments.

@gfyoung
Copy link
Member

gfyoung commented Dec 24, 2018

@jreback : Tests are all green. PTAL.

@jbrockmendel : Any other thoughts?

@gfyoung
Copy link
Member

gfyoung commented Dec 27, 2018

Alrighty, I think we have enough approvals here 😉

@gfyoung gfyoung merged commit 3086e0a into pandas-dev:master Dec 27, 2018
@gfyoung
Copy link
Member

gfyoung commented Dec 27, 2018

Thanks @NasaGeek for contributing to this!

thoo added a commit to thoo/pandas that referenced this pull request Dec 28, 2018
@thoo
Merge remote-tracking branch 'upstream/master' into series-dtype-doc
63c2aef 
* upstream/master: (26 commits)
  DOC: Fixing doc upload (no such remote origin) (pandas-dev#24459)
  BLD: for C extension builds on mac, target macOS 10.9 where possible (pandas-dev#24274)
  POC: _eadata (pandas-dev#24394)
  DOC: Correct location (pandas-dev#24456)
  CI: Moving CircleCI build to Travis (pandas-dev#24449)
  BUG: Infer compression by default in read_fwf() (pandas-dev#22200)
  DOC: Fix minor typo in whatsnew (pandas-dev#24453)
  DOC: Add dateutil to intersphinx pandas-dev#24437 (pandas-dev#24443)
  DOC: Adding links to offset classes in timeseries.rst (pandas-dev#24448)
  DOC: Adding offsets to API ref (pandas-dev#24446)
  DOC: fix flake8 issue in groupby.rst (pandas-dev#24363)
  DOC: Fixing more doc warnings (pandas-dev#24438)
  API: Simplify repeat signature (pandas-dev#24447)
  BUG: to_datetime(Timestamp, utc=True) localizes to UTC (pandas-dev#24441)
  CLN: Cython Py2/3 Compatible Imports (pandas-dev#23940)
  DOC: Fixing more doc warnings (pandas-dev#24431)
  DOC: Removing old release.rst (pandas-dev#24427)
  BUG-24408 Series.dt does not maintain own copy of index (pandas-dev#24426)
  DOC: Fixing several doc warnings (pandas-dev#24430)
  ENH: fill_value argument for shift pandas-dev#15486 (pandas-dev#24128)
  ...
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@NasaGeek @Pingviinituutti
BUG: Infer compression by default in read_fwf() (pandas-dev#22200)
05e1eae 
Closes pandas-devgh-22199.
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@NasaGeek @Pingviinituutti
BUG: Infer compression by default in read_fwf() (pandas-dev#22200)
a2b99a7 
Closes pandas-devgh-22199.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gfyoung gfyoung gfyoung left review comments

@jbrockmendel jbrockmendel jbrockmendel requested changes

@jreback jreback jreback approved these changes

@mroeschke mroeschke mroeschke approved these changes

Assignees
No one assigned
Labels
Bug Compat pandas objects compatability with Numpy or Python functions IO Data IO issues that don't fit into a more specific label
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.

read_fwf() function does not infer compression by default
6 participants
@NasaGeek @jreback @pep8speaks @gfyoung @jbrockmendel @mroeschke