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.

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

Fix #91 (spec_date_to_iso8601) #93

Merged
merged 2 commits into from
Jun 2, 2016
Merged

Conversation

dnaudet
Copy link
Contributor

@dnaudet dnaudet commented Jun 1, 2016

Fix for #91
Added unit tests for spec_date_to_iso8601.
Nominal tests only.
The zone keyword is not tested. I haven't added tests for the error cases either.
I checked that the new tests fail with the previous version of the function.

BTW the function only works if the provided spec dates are in english (Mon, Apr, Aug, Sun, ....). Are we sure this will always be the case?

@PiRK
Copy link
Member

PiRK commented Jun 2, 2016

I have investigated Jérôme's suggestion of using datetime. It could simplify the function, especially if we expect to add more date formats in the future. Initially I expected SPEC to write a single format.

>>> from datetime import datetime
>>> d = datetime.strptime('Thu Feb 11 09:54:35+0100 2016', '%a %b %d %H:%M:%S%z %Y')
>>> d.isoformat()
'2016-02-11T09:54:35+01:00'
>>> 
>>> d = datetime.strptime('Sat 2015/03/14 03:53:50+0200', '%a %Y/%m/%d %H:%M:%S%z')
>>> d.isoformat()
'2015-03-14T03:53:50+02:00'

Des anyone have any objection to rewriting the entire function?

@dnaudet
Copy link
Contributor Author

dnaudet commented Jun 2, 2016

I think this works only if you set the locale.LC_TIME right.

e.g :

d = datetime.strptime('Mo Feb 11 09:54:35 2016', '%a %b %d %H:%M:%S %Y')

doesn't work if the computer's locale is set to german (locale.setlocale(locale.LC_TIME, 'de_DE.UTF-8'))
It expects this :

d = datetime.strptime('Do Feb 11 09:54:35 2016', '%a %b %d %H:%M:%S %Y')

@PiRK PiRK merged commit 2d549ad into silx-kit:master Jun 2, 2016
@dnaudet dnaudet deleted the fix_date_conv branch July 4, 2016 12:14
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.

None yet

2 participants