CLN: Enforce read_csv(keep_date_col, parse_dates) deprecations #58622
Conversation
| cols_needed = [] | ||
|
|
||
| cols_needed = list(cols_needed) | ||
| if not isinstance(self.parse_dates, list): |
There was a problem hiding this comment.
This branch feels a bit odd in this method - this isn't handled by the caller at all?
There was a problem hiding this comment.
Yeah appears not. I suppose since this is called by the c parser and python parser maybe this was here to consolidate logic?
| list(columns), self.index_col | ||
| ) | ||
| self._name_processed = True | ||
| date_index = self._get_complex_date_index(data, columns) |
There was a problem hiding this comment.
can _get_complex_date_index be removed?
| warnings.warn( | ||
| "The 'keep_date_col' keyword in pd.read_csv is deprecated and " | ||
| "will be removed in a future version. Explicitly remove unwanted " | ||
| "columns after parsing instead.", |
There was a problem hiding this comment.
i think i screwed up this deprecation. the message here suggests that the post-deprecation behavior will retain the un-parsed column, which i think is not the case.
There was a problem hiding this comment.
Yeah I don't think the workaround messaging was quite right, but I think it's OK since we didn't get any issues AFAICT where people were confused by this.
There was a problem hiding this comment.
Plus this keyword only really made sense when parse_dates specified multiple columns which was clearly deprecated
There was a problem hiding this comment.
sounds good, thanks for taking a look
| depr = True | ||
| if depr: | ||
| warnings.warn( | ||
| "Support for nested sequences for 'parse_dates' in pd.read_csv " |
There was a problem hiding this comment.
did you keep any tests that current get here?
There was a problem hiding this comment.
Yeah there are tests where parse_dates is a list and valid and a list and not valid
|
Going to merge this in. Happy to follow up if needed |
xref #56569