Skip to content
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

Jump to bottom

Fix: DataFrame created with tzinfo cannot use to_dict(orient="records) #18416

Merged
merged 1 commit into from
Nov 23, 2017
Merged

Fix: DataFrame created with tzinfo cannot use to_dict(orient="records) #18416

merged 1 commit into from
Nov 23, 2017

Conversation

bolkedebruin
Copy link
Contributor

Columns with datetimez are not returning arrays.

Closes #18372

@jreback jreback added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode Timezones Timezone data dtype labels Nov 22, 2017
jreback
jreback requested changes Nov 22, 2017
View reviewed changes
doc/source/whatsnew/v0.22.0.txt Outdated
@@ -108,7 +108,7 @@ Bug Fixes
Conversion
^^^^^^^^^^

-
- Bug in :func:`DataFrame.to_dict` where columns of datetimez where not arrays when used with ``orient='records'`` (:issue:`18372`)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make this a bit more clear

pandas/tests/tseries/test_timezones.py Outdated
@@ -882,6 +882,17 @@ def test_frame_reset_index(self):
rs = roundtripped.index.tz
assert xp == rs

def test_frame_to_dict_tz(self):
data = [(datetime(2017, 11, 18, 21, 53, 0, 219225, tzinfo=pytz.utc),),
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue number as a comment.

this test goes in pandas/tests/frames/test_convert_to.py

pandas/tests/tseries/test_timezones.py Outdated
(datetime(2017, 11, 18, 22, 6, 30, 61810, tzinfo=pytz.utc,),)]
df = DataFrame(list(data), columns=["datetime", ])

rec = df.to_dict(orient='records')
Copy link
Contributor

Choose a reason for hiding this comment

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

result =

pandas/tests/tseries/test_timezones.py Outdated
df = DataFrame(list(data), columns=["datetime", ])

rec = df.to_dict(orient='records')
t1 = Timestamp('2017-11-18 21:53:00.219225+0000', tz=pytz.utc)
Copy link
Contributor

Choose a reason for hiding this comment

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

construct the dict and compare, call it expected

pandas/core/frame.py
@@ -993,7 +993,7 @@ def to_dict(self, orient='dict', into=dict):
for k, v in compat.iteritems(self))
elif orient.lower().startswith('r'):
return [into_c((k, _maybe_box_datetimelike(v))
for k, v in zip(self.columns, row))
for k, v in zip(self.columns, np.atleast_1d(row)))
for row in self.values]
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the np.atleast_2d be around self.values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it fails then. In that case only one of the results will be returned, effectively flattening the array.

jreback
jreback requested changes Nov 22, 2017
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

minor comments, ping when pushed / green.

pandas/tests/frame/test_convert_to.py Outdated
@@ -249,3 +252,16 @@ def test_to_dict_box_scalars(self):

result = DataFrame(d).to_dict(orient='records')
assert isinstance(result[0]['a'], (int, long))

def test_frame_to_dict_tz(self):
data = [(datetime(2017, 11, 18, 21, 53, 0, 219225, tzinfo=pytz.utc),),
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue number here as a comment.

doc/source/whatsnew/v0.22.0.txt Outdated
@@ -108,7 +108,7 @@ Bug Fixes
Conversion
^^^^^^^^^^

-
- Bug in :func:`DataFrame.to_dict` where columns of datetimez were not converted to required arrays when used with ``orient='records'``, raising``TypeError` (:issue:`18372`)
Copy link
Contributor

Choose a reason for hiding this comment

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

say columns of datetime that are tz-aware, were not ......

Copy link
Contributor

Choose a reason for hiding this comment

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

you can move this to 0.21.1

jreback
jreback requested changes Nov 22, 2017
View reviewed changes
doc/source/whatsnew/v0.22.0.txt Outdated
@@ -108,7 +108,7 @@ Bug Fixes
Conversion
^^^^^^^^^^

-
- Bug in :func:`DataFrame.to_dict` where columns of datetimez were not converted to required arrays when used with ``orient='records'``, raising``TypeError` (:issue:`18372`)
Copy link
Contributor

Choose a reason for hiding this comment

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

you can move this to 0.21.1

@codecov
Copy link

codecov bot commented Nov 23, 2017 •
edited
Loading

Codecov Report

Merging #18416 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18416      +/-   ##
==========================================
- Coverage   91.35%   91.34%   -0.02%     
==========================================
  Files         163      163              
  Lines       49691    49691              
==========================================
- Hits        45397    45388       -9     
- Misses       4294     4303       +9
Flag Coverage Δ
#multiple 89.14% <ø> (ø) ⬆️
#single 39.67% <ø> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/core/frame.py 97.8% <ø> (-0.1%) ⬇️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c634352...e861d08. Read the comment docs.

@pep8speaks
Copy link

pep8speaks commented Nov 23, 2017 •
edited
Loading

Hello @bolkedebruin! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on November 23, 2017 at 13:10 Hours UTC

@bolkedebruin
[BUG-FIX] DataFrame created with tzinfo cannot use to_dict(orient="re…
e861d08 
…cords")

Columns with datetimez are not returning arrays.

Closes pandas-dev#18372
@jreback jreback added this to the 0.21.1 milestone Nov 23, 2017
@jreback jreback merged commit 4e09480 into pandas-dev:master Nov 23, 2017
@jreback
Copy link
Contributor

jreback commented Nov 23, 2017

thanks @bolkedebruin

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Dec 8, 2017
@bolkedebruin @TomAugspurger
[BUG-FIX] DataFrame created with tzinfo cannot use to_dict(orient="re…
7f9dac8 
…cords") (pandas-dev#18416)

Closes pandas-dev#18372

(cherry picked from commit 4e09480)
TomAugspurger pushed a commit that referenced this pull request Dec 11, 2017
@bolkedebruin @TomAugspurger
[BUG-FIX] DataFrame created with tzinfo cannot use to_dict(orient="re…
eee26b2 
…cords") (#18416)

Closes #18372

(cherry picked from commit 4e09480)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode Timezones Timezone data dtype
Projects
None yet
Milestone
0.21.1
Development

Successfully merging this pull request may close these issues.

BUG: DataFrame created with tzinfo cannot use to_dict(orient="records")
4 participants
@bolkedebruin @pep8speaks @jreback @TomAugspurger