Skip to content

Fix unrecognized 'Z' UTC designator #8786

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

Closed
wants to merge 1 commit into from

Conversation

broessli
Copy link

closes #8771

@jreback jreback added Regression Functionality that used to work in a prior pandas version Datetime Datetime data dtype labels Nov 11, 2014
@jreback jreback added this to the 0.15.2 milestone Nov 11, 2014
@jreback
Copy link
Contributor

jreback commented Nov 11, 2014

ok, looks simple enough! cross-fingers other tests pass!
pls add a release note as well (referencing the original issue) (put in bug-fixes, though you can say regression)

@@ -363,7 +363,7 @@ convert_datetimestruct_local_to_utc(pandas_datetimestruct *out_dts_utc,
* to be cast to the 'unit' parameter.
*
* 'out' gets filled with the parsed date-time.
* 'out_local' gets whether returned value contains timezone. 0 for UTC, 1 for local time.
* 'out_local' gets whether returned value contains timezone.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe change the comment to explain the 0/1 return

@broessli
Copy link
Author

Hi, do you need me to do other modifications before merging? Let me know

@jreback
Copy link
Contributor

jreback commented Nov 14, 2014

looks good. pls rebase and squash. ping when green.

@broessli
Copy link
Author

done!

@jreback
Copy link
Contributor

jreback commented Nov 14, 2014

merged via c03e92f

@jreback jreback closed this Nov 14, 2014
@jreback
Copy link
Contributor

jreback commented Nov 14, 2014

@broessli hmm, I think this was changed before the latest travis build. Thought it was green.

Can you see what the problem is and and do a follow up PR? thanks (pls submit a new PR, can't push to this one anymore)

https://travis-ci.org/pydata/pandas/builds/41039139

@jreback jreback reopened this Nov 14, 2014
@broessli
Copy link
Author

Yes, sorry about that. The issue seems related to dateutil behavior, which is the fallback when parsing dates with ...Z00, depending on the version we can get a time zone equal to tzutc() or tzlocal(). Not sure how to handle this (besides dropping the test on Z00...)

@broessli
Copy link
Author

By the way, checking ISO 8601 I am not sure what to expect exactly with Z00, is it really legit?

@jreback
Copy link
Contributor

jreback commented Nov 14, 2014

so it give one answer on dateutil 1.5 and a different on 2.1/2.2?

you can do the test like this:

from distutils.version import LooseVersion

.....


if LooseVersion(dateutil.__version__) >= '2.2':
    #.....
else:
    # ....

I am not sure of the defined behavior of ...Z00

cc @rockg ?

@broessli
Copy link
Author

Actually, testing on my machine, I always get tzutc() regardless of dateutil's version, cannot figure out why this is different on Travis

@jreback
Copy link
Contributor

jreback commented Nov 14, 2014

hmm, I think this might have to do with the cache not updating because only the .c files were changed (and not the cython files). let me see.

@jreback
Copy link
Contributor

jreback commented Nov 15, 2014

reverted for now: 6f48d4a

got ahead and rebase on master and put up a new PR (close this one)

@jreback jreback closed this Nov 15, 2014
@broessli broessli deleted the fix-z-designator branch November 16, 2014 14:09
@jreback
Copy link
Contributor

jreback commented Nov 16, 2014

cc @dbew

any thoughts on the above?

https://travis-ci.org/pydata/pandas/builds/41039139

@dbew
Copy link
Contributor

dbew commented Nov 16, 2014

Nothing jumps out, I'm afraid.

There were definitely differences between python 3 and python 2 when I did the original dateutil work but the issues I had were all around UTC and the implementation of the timezones, not tzlocal and parsing.

Looking at the parse code in dateutil, I think it reads the timezone name and first tests if it matches the local timezone (tzname in time.tzname) then tests if it matches UTC - so if 'Z' matches the local timezone then it wouldn't be reported at UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Regression Functionality that used to work in a prior pandas version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timestamp does not parse the 'Z' zone designator for UTC
3 participants