Skip to content

BUG: Fix using dtype with parse_dates in read_csv #34330

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.

Sign up for GitHub

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 19 commits into from
Closed

BUG: Fix using dtype with parse_dates in read_csv #34330

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c94b45e
PERF: Remove unnecessary copies in sorting functions
mproszewska May 15, 2020
0ab450b
Run tests
mproszewska May 16, 2020
54c7304
Run tests
mproszewska May 16, 2020
6d72a34
Add asv
mproszewska May 22, 2020
5ba54a6
Run black
mproszewska May 22, 2020
2766270
Remove asv
mproszewska May 22, 2020
b800207
BUG: Fix using dtype with parse_dates in read_csv
mproszewska May 23, 2020
6b8f562
Fix lint
mproszewska May 23, 2020
91176ca
Merge branch 'perf'
mproszewska May 24, 2020
f748b78
Merge remote-tracking branch 'upstream/master'
mproszewska May 28, 2020
3c13f59
Modify test
mproszewska May 28, 2020
c04c494
Merge remote-tracking branch 'upstream/master'
mproszewska Jun 1, 2020
d9aa319
Add asv
mproszewska Jun 5, 2020
0afb1b1
Fix
mproszewska Jun 5, 2020
85dd0d6
Run black
mproszewska Jun 8, 2020
4b53bf7
Merge branch 'master' into parse-dates-dtype
mproszewska Oct 8, 2020
b3d5ca5
Merge branch 'master' into parse-dates-dtype
mproszewska Oct 8, 2020
4175b80
Run black
mproszewska Oct 8, 2020
6a78220
Run isort
mproszewska Oct 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pandas/io/parsers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1708,7 +1708,9 @@ def _convert_to_ndarrays(
result = {}
for c, values in dct.items():
conv_f = None if converters is None else converters.get(c, None)
if isinstance(dtypes, dict):
if values.dtype != object:
Copy link
Contributor

Choose a reason for hiding this comment

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

this is very odd to do as we already have a path for a single dtype, what are you trying to do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After loading values from csv we have dictionary with column names and numpy array for each column with dtype=object. Then we change values that are suppose to be datetime ('b' in example). After that we want to change types of the rest of columns, that is those that have dtype=object. In that line we're skiping columns that already have dtype set.

Copy link
Member

Choose a reason for hiding this comment

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

Use is_object_dtype(values.dtype) to do the check

cast_type = values.dtype
elif isinstance(dtypes, dict):
cast_type = dtypes.get(c, None)
else:
# single dtype or None
Expand Down Expand Up @@ -3264,6 +3266,9 @@ def _make_date_converter(
):
def converter(*date_cols):
if date_parser is None:
date_cols = tuple(
x if isinstance(x, np.ndarray) else x.to_numpy() for x in date_cols
)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is better off done inside concat_date_cols, but what is the incoming data here in the example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's tuple with StringArray with dates from 'b'.

Copy link
Member

Choose a reason for hiding this comment

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

is it possible to move this to concat_date_cols as per @jreback comment?

strs = parsing.concat_date_cols(date_cols)

try:
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/io/parser/test_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2135,3 +2135,17 @@ def test_no_header_two_extra_columns(all_parsers):
parser = all_parsers
df = parser.read_csv(stream, header=None, names=column_names, index_col=False)
tm.assert_frame_equal(df, ref)


def test_dtype_with_parse_dates(all_parsers):
# GH 34066
parser = all_parsers
data = """
a,b
1,2020-05-23 01:00:00"""
expected = DataFrame(
[["1", "2020-05-23 01:00:00"]], columns=["a", "b"], dtype="string"
)
expected = expected.astype({"b": np.datetime64})
Copy link
Contributor

Choose a reason for hiding this comment

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

this is very strange to astype this way, please use pd.to_datetime

Copy link
Member

Choose a reason for hiding this comment

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

@dsaxton's suggestion above to declare types in the constructor would remove the need to do any type-casting.

df = parser.read_csv(StringIO(data), dtype="string", parse_dates=["b"])
tm.assert_frame_equal(df, expected)
Copy link
Member

Choose a reason for hiding this comment

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

Can you call this result instead of df? Also instead of astyping expected you should be able to set the types in the constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only one type is allowed in constructor, so I'm not sure how I can set it there