Skip to content

FIX # 45957 issue read_csv with empty list #46033

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

Closed
wants to merge 1 commit into from

Conversation

profMsaif
Copy link

@pep8speaks
Copy link

Hello @profMsaif! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 1206:9: E265 block comment should start with '# '
Line 1206:69: W291 trailing whitespace
Line 1207:9: E265 block comment should start with '# '

@twoertwein
Copy link
Member

I think TextParser can call TextFileReader with a list but TextParser always uses the python engine. I would recommend adding something like this after the if block:

elif engine != "python":
    raise TypeError("Lists are only supported with engine='python'.")

Probably need to phrase it differently: I don't think we want to encourage people calling read_csv with lists.

@profMsaif
Copy link
Author

I think TextParser can call TextFileReader with a list but TextParser always uses the python engine. I would recommend adding something like this after the if block:

elif engine != "python":
    raise TypeError("Lists are only supported with engine='python'.")

Probably need to phrase it differently: I don't think we want to encourage people calling read_csv with lists.

This message will be not informative for the pandas user,
As read_csv expect file of file path or a iobuffer
List or any other object should never be a valid value for this variable.
The behaviour of pandas older version like (1.2.* or 1.3.*) was with ValueError , no matter what you pass dict list tuple it will always raise ValueError and I think this behaviour shouldn’t changed

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

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

I am pretty sure, that a list is valid here. I think we get a list when calling the Parsers through read_excel.

Please always add tests

Edit: Yep confirmed, read_excel passes through here with a list

@twoertwein
Copy link
Member

twoertwein commented Feb 18, 2022

Then how about raising exactly the same error as in previous versions?

elif engine != "python":
    # read_excel uses the python engine with a list
    raise ValueError(f"Invalid file path or buffer object type: {type(f)}")

Please don't forget to also add a test case.

edit:

Or could have something like this

if isinstance(filepath_or_buffer, list):
     ValueError(f"Invalid file path or buffer object type: {type(f)}")

around here

"""Generic reader of line files."""

@jreback jreback added the IO CSV read_csv, to_csv label Feb 27, 2022
@simonjayhawkins
Copy link
Member

closing as fixed by #46325. Thanks @profMsaif for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO CSV read_csv, to_csv
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passing an empty list to read_csv causes segmentation fault
7 participants