BUG: Fix to_dict() problem when using datetime DataFrame #11247 by ethanluoyc · Pull Request #11327 · pandas-dev/pandas (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation10 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

ethanluoyc

closes #11247

This is my first time contributing through PR so forgive me if I make any silly mistake.
The issue can be referred to in GH11247
I followed @jreback 's suggestion to box things up in to_dict() but I am not sure if this is what he means because I do not know exactly when _maybe_datetime() should be used. Initially I tried to tweak internals.py but it fails.

Let me know how I can improve from here. Hope I can really learn from my first PR.

@jreback

@ethanluoyc

I think I forgot to fetch to update my local master before my first commit to this PR. Somehow I messed up is it true?

@jreback

add tests! (ideally for each one of the orients if we dont' have already).

@ethanluoyc

which test case shall I add in? I am new to this :)

On 15 Oct 2015, at 2:50 AM, Jeff Reback notifications@github.com wrote:

add tests! (ideally for each one of the orients if we dont' have already).


Reply to this email directly or view it on GitHub #11327 (comment).

@ethanluoyc

jreback

def test_to_dict_timestamp(self):
# GH11247
tsmp = Timestamp('20130101')
test_data = DataFrame({'A': [tsmp, tsmp], 'B': [tsmp, tsmp]})

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 mixed frame ,e.g. add a float and a string say. obviously need to adjust the tests as well.

Choose a reason for hiding this comment

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

I adjusted the tests and now it tests both mixed and single dtypes

@jreback

pls add a whatsnew note for 0.17.1 in bug fixes

@ethanluoyc

Just asking what I should do if I merge updates locally with my branch and need to merge the 2 commits into one..


Sent from Mailbox

On Fri, Oct 16, 2015 at 6:25 AM, Jeff Reback notifications@github.com
wrote:

pls add a whatsnew note for 0.17.1 in bug fixes

Reply to this email directly or view it on GitHub:
#11327 (comment)

@ethanluoyc

Fix a bug where to_dict() does not return Timestamp when there is only datetime dtype present.

@jreback

2 participants

@ethanluoyc @jreback