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

Parse lower-cased 'h' #190

Merged
merged 2 commits into from Jan 22, 2015
Merged

Parse lower-cased 'h' #190

merged 2 commits into from Jan 22, 2015

Conversation

tamentis
Copy link
Contributor

@tamentis tamentis commented Jan 6, 2015

It appears the documented 'h' and 'hh' tokens are not currently working, the following Pull Request is an attempt at fixing that.

Before the fix:

>>> arrow.get("5 PM", "h A")
<Arrow [0001-01-01T12:00:00+00:00]>

After the fix:

>>> arrow.get("5 PM", "h A")
<Arrow [0001-01-01T17:00:00+00:00]>

@@ -175,7 +175,7 @@ def _parse_token(self, token, value, parts):
elif token in ['DD', 'D']:
parts['day'] = int(value)

elif token in ['HH', 'H']:
elif token in ['HH', 'H', 'hh', 'h']:
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be just as easy to do a token.upper() now?

It could then be applied to the DD/D case just above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that would break 's' and 'S'.

Copy link
Contributor

Choose a reason for hiding this comment

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

On Wed, Jan 07, 2015 at 05:37:44PM -0800, Bertrand Janin wrote:

In arrow/parser.py:

@@ -175,7 +175,7 @@ def _parse_token(self, token, value, parts):
elif token in ['DD', 'D']:
parts['day'] = int(value)

  •    elif token in ['HH', 'H']:
    
  •    elif token in ['HH', 'H', 'hh', 'h']:
    

But that would break 's' and 'S'.

Depends if you do at the top an token = token.upper() or just adjust the
if/elif statements to be elif token.upper() in ['HH', 'H']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated that as per your comment, however I can't use the same logic for dd/d since they can't be parsed the same way as DD/D (ints VS names). I'll give that a shot in a separate pull request.

andrewelkins added a commit that referenced this pull request Jan 22, 2015
@andrewelkins andrewelkins merged commit 5f678e1 into arrow-py:master Jan 22, 2015
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1624fd2 on truveris:master into * on crsmithdev:master*.

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

4 participants