BUG: df.to_csv() fails to a not-yet-created file when the path is fsspec-based (#55828) #56309
Conversation
When specifying local to_csv file paths with the file scheme, Pandas will now create the file instead of raising an exception
When specifying local to_csv file paths with the file scheme, Pandas will now create the file instead of raising an exception
When specifying local to_csv file paths with the file scheme, Pandas will now create the file instead of raising an exception
When specifying local to_csv file paths with the file scheme, Pandas will now create the file instead of raising an exception
When specifying local to_csv file paths with the file scheme, Pandas will now create the file instead of raising an exception
| file_path = urllib.request.url2pathname(parsed_url.path) | ||
| file_path = os.path.normpath(file_path) | ||
| return IOArgs( | ||
| filepath_or_buffer=open(file_path, "rb"), |
There was a problem hiding this comment.
Shouldn't this still respect mode?
| @@ -382,6 +382,19 @@ def _get_filepath_or_buffer( | |||
| # urlopen function defined elsewhere in this module | |||
| import urllib.request | |||
|
|
|||
| # Fix for GH #55828 | |||
| parsed_url = parse_url(filepath_or_buffer) | |||
There was a problem hiding this comment.
I believe is_url should not be true for fsspec urls. So that might be a much nicer way of fixing this issue (I think @krehm was also hinting at that in the issue) - I'm not familiar with the urllib regex, we might need to exclude more fsspec URLs from it.
There was a problem hiding this comment.
@twoertwein it did seem to me that any fsspec url in is_url is guaranteed to fail in this case, which seemed like a logic flaw to me. But I'm not familiar with the urllib code either, so was hesitant to specify a particular solution.
There was a problem hiding this comment.
The overlap between urllib/fsspec is at the moment:
>>> set(uses_relative + uses_netloc + uses_params).intersection(fsspec.available_protocols())
{'sftp', 'ftp', 'file', 'git', 'http', 'https'}Could have something like this:
_VALID_URLS = set(uses_relative + uses_netloc + uses_params).difference( fsspec.available_protocols())
_VALID_URLS.update(("http", "https"))
_VALID_URLS.discard("")Technically this is a behavior change for sftp, git, ... (might be okay, probably not used frequently?). fsspec should have available_protocols since early 2022 fsspec/filesystem_spec#913 might need to double check whether we need to bump the minimum version of fsspec. This might make the regex in is_fsspec_url obsolete.
There was a problem hiding this comment.
@twoertwein I didn't want to touch is_url as its used in a few other places and I wasn't sure if it would break anything. Is it okay to do so?
These are the places is_url is used without a corresponding is_fsspec_url call:
pandas.io.html._LxmlFrameParser._build_doc
pandas.io.html._read
pandas.io.formats.html.HTMLFormatter._write_cell
There was a problem hiding this comment.
Thank you for checking that!
Do you think it is possible to replace if isinstance(filepath_or_buffer, str) and is_url(filepath_or_buffer): with if isinstance(filepath_or_buffer, str) and is_url(filepath_or_buffer) and not is_fsspec_url(filepath_or_buffer):?
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen. |
When specifying local to_csv file paths with the file scheme, Pandas will now create the file instead of raising an exception
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.