BUG: Delegate more of Excel parsing to CSV #23544
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hello @gfyoung! Thanks for submitting the PR.
|
gfyoung
force-pushed
the
excel-csv-standardize
branch
from
9be00cb
to
56244c8
Compare
|
wow nice cleanup! linting check import format using isort |
gfyoung
force-pushed
the
excel-csv-standardize
branch
from
56244c8
to
bb8825f
Compare
Codecov Report
@@ Coverage Diff @@
## master #23544 +/- ##
==========================================
- Coverage 92.24% 92.23% -0.01%
==========================================
Files 161 161
Lines 51278 51278
==========================================
- Hits 47299 47298 -1
- Misses 3979 3980 +1
Continue to review full report at Codecov.
|
|
@jreback : Fixed the import error, so ready for review. |
gfyoung
force-pushed
the
excel-csv-standardize
branch
from
bb8825f
to
fc27613
Compare
4 tasks
gfyoung
force-pushed
the
excel-csv-standardize
branch
2 times, most recently
from
ddbc258
to
487a336
Compare
|
@jreback : Addressed comments, and all is still green. PTAL. |
jreback
requested changes
Nov 10, 2018
gfyoung
force-pushed
the
excel-csv-standardize
branch
2 times, most recently
from
113bc37
to
0e12260
Compare
|
@jreback : Need to rebase this onto |
|
@jreback : Comments addressed, PR rebased, and all is green. PTAL. |
The idea is that we read the Excel file, get the data, and then let the TextParser handle the reading and parsing. We shouldn't be doing a lot of work that is already defined in parsers.py In doing so, we identified several bugs: * index_col=None was not being respected * usecols behavior was inconsistent with that of read_csv for list of strings and callable inputs * usecols was not being validated as proper Excel column names when passed as a string. Closes pandas-devgh-18273. Closes pandas-devgh-20480.
gfyoung
force-pushed
the
excel-csv-standardize
branch
from
0e12260
to
a36aa76
Compare
jreback
approved these changes
Nov 11, 2018
thoo
added a commit
to thoo/pandas
that referenced
this pull request
Nov 11, 2018
* upstream/master: BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index (pandas-dev#23524) BUG: Delegate more of Excel parsing to CSV (pandas-dev#23544) API: DataFrame.__getitem__ returns Series for sparse column (pandas-dev#23561) CLN: use float64_t consistently instead of double, double_t (pandas-dev#23583) DOC: Fix Order of parameters in docstrings (pandas-dev#23611) TST: Unskip some Categorical Tests (pandas-dev#23613) TST: Fix integer ops comparison test (pandas-dev#23619) DOC: Fixes to docstring to add validation to CI (pandas-dev#23560) DOC: Remove incorrect periods at the end of parameter types (pandas-dev#23600) MAINT: tm.assert_raises_regex --> pytest.raises (pandas-dev#23592) DOC: Updating Series.resample and DataFrame.resample docstrings (pandas-dev#23197)
thoo
added a commit
to thoo/pandas
that referenced
this pull request
Nov 12, 2018
…fixed * upstream/master: DOC: Enhancing pivot / reshape docs (pandas-dev#21038) TST: Fix xfailing DataFrame arithmetic tests by transposing (pandas-dev#23620) BUILD: Simplifying contributor dependencies (pandas-dev#23522) BUG/REF: TimedeltaIndex.__new__ (pandas-dev#23539) BUG: Casting tz-aware DatetimeIndex to object-dtype ndarray/Index (pandas-dev#23524) BUG: Delegate more of Excel parsing to CSV (pandas-dev#23544) API: DataFrame.__getitem__ returns Series for sparse column (pandas-dev#23561) CLN: use float64_t consistently instead of double, double_t (pandas-dev#23583) DOC: Fix Order of parameters in docstrings (pandas-dev#23611) TST: Unskip some Categorical Tests (pandas-dev#23613) TST: Fix integer ops comparison test (pandas-dev#23619)
gfyoung
added a commit
to forking-repos/pandas
that referenced
this pull request
Nov 12, 2018
gfyoung
added a commit
to forking-repos/pandas
that referenced
this pull request
Nov 12, 2018
jreback
pushed a commit
that referenced
this pull request
Nov 12, 2018
JustinZhengBC
pushed a commit
to JustinZhengBC/pandas
that referenced
this pull request
Nov 14, 2018
The idea is that we read the Excel file, get the data, and then let the TextParser handle the reading and parsing. We shouldn't be doing a lot of work that is already defined in parsers.py In doing so, we identified several bugs: * index_col=None was not being respected * usecols behavior was inconsistent with that of read_csv for list of strings and callable inputs * usecols was not being validated as proper Excel column names when passed as a string. Closes pandas-devgh-18273. Closes pandas-devgh-20480.
JustinZhengBC
pushed a commit
to JustinZhengBC/pandas
that referenced
this pull request
Nov 14, 2018
tm9k1
pushed a commit
to tm9k1/pandas
that referenced
this pull request
Nov 19, 2018
The idea is that we read the Excel file, get the data, and then let the TextParser handle the reading and parsing. We shouldn't be doing a lot of work that is already defined in parsers.py In doing so, we identified several bugs: * index_col=None was not being respected * usecols behavior was inconsistent with that of read_csv for list of strings and callable inputs * usecols was not being validated as proper Excel column names when passed as a string. Closes pandas-devgh-18273. Closes pandas-devgh-20480.
tm9k1
pushed a commit
to tm9k1/pandas
that referenced
this pull request
Nov 19, 2018
Pingviinituutti
pushed a commit
to Pingviinituutti/pandas
that referenced
this pull request
Feb 28, 2019
The idea is that we read the Excel file, get the data, and then let the TextParser handle the reading and parsing. We shouldn't be doing a lot of work that is already defined in parsers.py In doing so, we identified several bugs: * index_col=None was not being respected * usecols behavior was inconsistent with that of read_csv for list of strings and callable inputs * usecols was not being validated as proper Excel column names when passed as a string. Closes pandas-devgh-18273. Closes pandas-devgh-20480.
Pingviinituutti
pushed a commit
to Pingviinituutti/pandas
that referenced
this pull request
Feb 28, 2019
Pingviinituutti
pushed a commit
to Pingviinituutti/pandas
that referenced
this pull request
Feb 28, 2019
The idea is that we read the Excel file, get the data, and then let the TextParser handle the reading and parsing. We shouldn't be doing a lot of work that is already defined in parsers.py In doing so, we identified several bugs: * index_col=None was not being respected * usecols behavior was inconsistent with that of read_csv for list of strings and callable inputs * usecols was not being validated as proper Excel column names when passed as a string. Closes pandas-devgh-18273. Closes pandas-devgh-20480.
Pingviinituutti
pushed a commit
to Pingviinituutti/pandas
that referenced
this pull request
Feb 28, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The idea is that we read the Excel file, get the data, and let the
TextParserhandle reading and parsing, or at least most of it. We shouldn't be doing a lot of work that is already defined inparsers.pyIn doing so, identified several bugs:
index_col=Nonewas not being respectedusecolsbehavior was inconsistent with that ofread_csvfor list of strings and callable inputsusecolswas not being validated as proper Excel column names when passed as a string.Closes #18273.
Closes #20480.
With regards to the latter issue, I believe this PR is a cleaner implementation that integrates the new
usecolsfunctionality without having to introduce a new parameter.