-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
REGR: ValueError raised when both prefix and names are set to None #42690
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.
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
Merged
Merged
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
e0553ab
REGR: ValueError raised when both prefix and names are set to None
lithomas1 f5cc78b
Merge branch 'master' of https://github.com/pandas-dev/pandas into pr…
lithomas1 8dc6abb
Merge branch 'master' into prefix-names-raises
lithomas1 f12b311
Update readers.py
lithomas1 e3c10e1
whitespace
lithomas1 75d7083
Update v1.3.1.rst
lithomas1 56fc7dc
Merge branch 'pandas-dev:master' into prefix-names-raises
lithomas1 2729d71
Update v1.3.2.rst
lithomas1 5006779
Update readers.py
lithomas1 be486b8
Update readers.py
lithomas1 9e64088
Merge branch 'master' into prefix-names-raises
jreback File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -505,11 +505,11 @@ def read_csv( | |
delimiter=None, | ||
# Column and Index Locations and Names | ||
header="infer", | ||
names=lib.no_default, | ||
names=None, | ||
index_col=None, | ||
usecols=None, | ||
squeeze=False, | ||
prefix=lib.no_default, | ||
prefix=None, | ||
mangle_dupe_cols=True, | ||
# General Parsing Configuration | ||
dtype: DtypeArg | None = None, | ||
|
@@ -603,11 +603,11 @@ def read_table( | |
delimiter=None, | ||
# Column and Index Locations and Names | ||
header="infer", | ||
names=lib.no_default, | ||
names=None, | ||
index_col=None, | ||
usecols=None, | ||
squeeze=False, | ||
prefix=lib.no_default, | ||
prefix=None, | ||
mangle_dupe_cols=True, | ||
# General Parsing Configuration | ||
dtype: DtypeArg | None = None, | ||
|
@@ -1224,8 +1224,8 @@ def _refine_defaults_read( | |
error_bad_lines: bool | None, | ||
warn_bad_lines: bool | None, | ||
on_bad_lines: str | None, | ||
names: ArrayLike | None | object, | ||
prefix: str | None | object, | ||
names: ArrayLike | None, | ||
prefix: str | None, | ||
defaults: dict[str, Any], | ||
): | ||
"""Validate/refine default values of input parameters of read_csv, read_table. | ||
|
@@ -1302,11 +1302,11 @@ def _refine_defaults_read( | |
if delimiter and (sep is not lib.no_default): | ||
raise ValueError("Specified a sep and a delimiter; you can only specify one.") | ||
|
||
if names is not lib.no_default and prefix is not lib.no_default: | ||
if names is not None and prefix is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.g here |
||
raise ValueError("Specified named and prefix; you can only specify one.") | ||
|
||
kwds["names"] = None if names is lib.no_default else names | ||
kwds["prefix"] = None if prefix is lib.no_default else prefix | ||
kwds["names"] = names | ||
kwds["prefix"] = prefix | ||
|
||
# Alias sep -> delimiter. | ||
if delimiter is None: | ||
|
This file contains hidden or 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would rather leave these as no_default and fix the check itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only have one way of representing none/null, so we should either use
None
orlib.no_default
, not have both. I think I preferNone
given thatlib.no_default
is private.If we want to keep using
lib.no_default
then, we should probably close OP as wontfix.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no i think you can simply revert these back to lib.no_default and fix the check on L1305. the point is that these are still invalid if actual
None
is passed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure iiuc, but
pd.read_csv(xxx, names=None, prefix=None)
should fail?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should work. It worked at least in previous versions. If
None
is passed for both, it would just get the column names from header, or use the "0,1,2,..." naming scheme, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right i agree this should work, but i also think you can leave the no_default and it will work (just odify the comparison)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thx for clarifying, update incoming soon.