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 parsing of localized meridians (refers #228) #244

Merged

Conversation

swistakm
Copy link
Contributor

changes

  • added tests for meridians with and without localization
  • a token now accepts only am/pm forms as stated previously
    in documentation
  • A token accepts both am/pm and AM/PM forms to ensure backwards
    compatibility
  • docs: updated output/accepted format of A & a tokens
  • added footnotes to some tokens explaining localization

Additional notes:

token A: existing tests ensured that A token accepts both lowercase and uppercase meridians. I decided to keep this behaviour to ensure backwards compatibility. I think this could be changed in future to accept only uppercase but this would require bumping of major version part.

token a: I have decided to drop support for a/A/p/P values from following reason:

  • single letter for instead of am/pm is a very rare case
  • this was already documented as a => am/pm, A => AM/PM
  • there is no support for single-letter meridians in locales.
  • during formatting a already translates to am/pminstead of a/p/A/P and such inconsistency should be treated as a major issue. So this needed to be made consistent across format/parse and I think the best option is to follow already documented behaviour.

documentation: we have few tokens that supports localization. I have added two footnotes to tokens table because this was not obvious if dates with those tokens can be parsed using locale. Additionally the footnote on ddd and dddd says that these tokens support only formatting since we do not allow such tokens in parse calls.

* added tests for meridians with and without localization
* `a` token now accepts only `am/pm` forms as stated previously
  in documentation
* `A` token accepts both `am/pm` and `AM/PM` forms to ensure backwards
  compatibility
* docs: updated output/accepted format of `A` & `a` tokens
* added footnotes to some tokens explaining localization
andrewelkins added a commit that referenced this pull request Jul 14, 2015
Fix parsing of localized meridians (refers #228) This is excellent. @swistakm
@andrewelkins andrewelkins merged commit 0e1ba6a into arrow-py:master Jul 14, 2015
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