Skip to content

BUG: wrong parsing of microseconds with format arg (#4152) #4166

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 9, 2013

Conversation

jorisvandenbossche
Copy link
Member

Closes #4152.

fraction was already in microseconds, so no need to multiply it with 1000000, which caused faulty parsing of dates with microseconds.

With this commit I get:

pandas version: 0.12.0.dev
Value to parse: 01-Apr-2011 00:00:01.978
datetime.strptime        : 2011-04-01 00:00:01.978000
to_datetime, w/out format: 2011-04-01 00:00:01.978000
to_datetime, w/ format   : 2011-04-01 00:00:01.978000

for

import datetime as dt
import pandas as pd
val = '01-Apr-2011 00:00:01.978'
print 'pandas version:',pd.__version__
print 'Value to parse:',val
format = '%d-%b-%Y %H:%M:%S.%f'
print 'datetime.strptime        :',dt.datetime.strptime(val, format)
print 'to_datetime, w/out format:',pd.to_datetime(val)
print 'to_datetime, w/ format   :', pd.to_datetime(val, format=format)

@jorisvandenbossche
Copy link
Member Author

I added a test based on the reported bug in the issue, but I don't have experience with writing tests. Is it OK like this?

@hayd
Copy link
Contributor

hayd commented Jul 8, 2013

Nearly, not sure this is going to work since now to_datetime returns a Timestamp object rather than a Datatime...

Maybe better to do (will also give nicer error message):

self.assertEquals(str(result),  '2011-04-01 00:00:01.978000')

@jreback
Copy link
Contributor

jreback commented Jul 8, 2013

that is fine

there is another issue in processing to_datetime
thanks for this catch

@jreback
Copy link
Contributor

jreback commented Jul 8, 2013

a Timestamp will compare to a datetime ok

@hayd
Copy link
Contributor

hayd commented Jul 8, 2013

@jreback my bad!

@jorisvandenbossche
Copy link
Member Author

OK!

@jreback What do you mean with another issue?

@jreback
Copy link
Contributor

jreback commented Jul 8, 2013

to_datetime had some convoluted logic and I came up with a case where the format was being interpreted correctly; the existing test was not good enough

but unrelated to the bug you are fixing.....

going to merge yours and then fix the other issue

@jreback
Copy link
Contributor

jreback commented Jul 8, 2013

you can reproduce if you pass a list of values (rather than a single value)

it will ignore the format

@jreback
Copy link
Contributor

jreback commented Jul 9, 2013

@jorisvandenbossche I take that back....looks like your PR fixes this....

can you add a release notes mention? (then squash together if you can)...thanks

@jorisvandenbossche
Copy link
Member Author

OK, I added something to the release notes. With 'squaqh together', do you mean putting it together in one commit?

I looked at the PR #4167 you mentioned, but I doubt that this PR fixes the test you added there ... (this one only changed something that had to do with microseconds)

@jorisvandenbossche
Copy link
Member Author

@jreback OK, I also put it in one commit.

@hayd
Copy link
Contributor

hayd commented Jul 9, 2013

lovely! Excellent work!

hayd added a commit that referenced this pull request Jul 9, 2013
BUG: wrong parsing of microseconds with format arg (#4152)
@hayd hayd merged commit 1a6b967 into pandas-dev:master Jul 9, 2013
@jreback
Copy link
Contributor

jreback commented Jul 9, 2013

@jorisvandenbossche 2nd that!

this really does fix the error, my PR just makes the test more consistent (and is indepdent of what you did)

thanks again

@cpcloud
Copy link
Member

cpcloud commented Jul 9, 2013

this introduced another bug it seems, see #4179

@jorisvandenbossche
Copy link
Member Author

@cpcloud Strange, it is exactly the test that I introduced in this PR and which should be fixed by this PR that is failing. Did you rebuild the cython files?

@cpcloud
Copy link
Member

cpcloud commented Jul 9, 2013

oh gosh duh. sorry. closing the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

to_datetime parsing bug when using format
4 participants