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

Updated regular expressions, fixed a few bugs, added tests for v0.15.0 #639

Merged
merged 19 commits into from
Sep 8, 2019
Merged

Conversation

jadchaar
Copy link
Member

  • X token can now properly parse float timestamp strings
  • Time strings in basic and extended format are now parsed using a regular expression
  • "Z" is no longer stripped from the end of the string to prevent mutating the passed in datetime string. I changed it to now split on the "Z" as well as the + and -, so the parsing logic now understands that a tz is present and will parse it accordingly (rather than going on the assumption that the logic defaults to UTC and ignores the Z, it is now more explicit with a tzutc object).
  • Removed stripping of trailing whitespace
  • Added a number of tests for regular expressions and the basic format
  • ParserError exceptions that were thrown when the day_of_year logic encountered an error were not shown to the user because they were being prematurely caught. I fixed this by adding a new ParserMatchError that inherits from ParserError that can be caught by the internal logic and allow the ParserErrors to propagate.
  • ParserError now inherits from ValueError, which makes more sense then a RuntimeError.
  • Removed warnings and incremented version.

@jadchaar jadchaar requested a review from systemcatch August 17, 2019 12:39
@jadchaar
Copy link
Member Author

Note: tests seem to be passing, but we are still not at 100% coverage.

arrow/_version.py Outdated Show resolved Hide resolved
# Before this, the ParserErrors were caught by the try/except in
# _parse_multiformat() and the appropriate error message was not
# transmitted to the user.
class ParserMatchError(ParserError):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea

arrow/parser.py Show resolved Hide resolved
arrow/parser.py Show resolved Hide resolved
arrow/parser.py Show resolved Hide resolved
has_minutes = colon_count == 1 or len(time_parts[0]) == 4
has_seconds = colon_count == 2 or len(time_parts[0]) == 6
has_subseconds = re.search("[.,]", time_parts[0])
time_components = self._TIME_RE.match(time_parts[0])
Copy link
Collaborator

Choose a reason for hiding this comment

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

👏 regex to the rescue again!

arrow/parser.py Show resolved Hide resolved
@@ -429,23 +441,23 @@ def _choice_re(choices, flags=0):


class TzinfoParser(object):

_TZINFO_RE = re.compile(r"([+\-])?(\d\d):?(\d\d)?")
# TODO: test against full timezone DB
Copy link
Collaborator

Choose a reason for hiding this comment

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

that's a good idea

arrow/parser.py Show resolved Hide resolved
docs/index.rst Show resolved Hide resolved
arrow/parser.py Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Aug 19, 2019

Codecov Report

Merging #639 into Version-0.15.0 will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff              @@
##           Version-0.15.0   #639   +/-   ##
=============================================
  Coverage             100%   100%           
=============================================
  Files                   8      8           
  Lines                1511   1507    -4     
  Branches              235    233    -2     
=============================================
- Hits                 1511   1507    -4
Impacted Files Coverage Δ
arrow/factory.py 100% <ø> (ø) ⬆️
arrow/parser.py 100% <100%> (ø) ⬆️
arrow/arrow.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed75554...32fe0a5. Read the comment docs.

tests/parser_tests.py Outdated Show resolved Hide resolved
self.assertEqual(self.parser.parse("01"), tz.tzoffset(None, 3600))
self.assertEqual(self.parser.parse("+01"), tz.tzoffset(None, 3600))
self.assertEqual(self.parser.parse("-01"), tz.tzoffset(None, -3600))

Copy link
Collaborator

Choose a reason for hiding this comment

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

💪

@jadchaar jadchaar mentioned this pull request Aug 28, 2019
Copy link
Collaborator

@systemcatch systemcatch left a comment

Choose a reason for hiding this comment

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

🚀

@jadchaar jadchaar merged commit 4a1c4c1 into arrow-py:Version-0.15.0 Sep 8, 2019
@jadchaar jadchaar deleted the tz-changes branch September 8, 2019 15:19
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.

3 participants