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

Bug fix/560: YY maps 2018 to 20 #580

Closed
wants to merge 4 commits into from
Closed

Conversation

dyerd
Copy link

@dyerd dyerd commented Apr 23, 2019

Potential bug fix for issues mentioned in #560. Enforces that year given for 'DD MMMM YY' is only 2 digits, and that year given for 'DD MMMM YYYY' is only 4 digits.

Per the comments on the issue report:

arrow.get('12 October 2018', 'DD MMMM YY') now raises an exception

and

arrow.get('01 January 123456789101112', 'DD MMMM YYYY') now raises an exception

@dyerd dyerd changed the title Bug fix/560 Bug fix/560: YY maps 2018 to 20 Apr 23, 2019
@codecov-io
Copy link

codecov-io commented Apr 23, 2019

Codecov Report

Merging #580 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #580   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          13     13           
  Lines        3399   3407    +8     
  Branches      238    239    +1     
=====================================
+ Hits         3399   3407    +8
Impacted Files Coverage Δ
arrow/arrow.py 100% <ø> (ø) ⬆️
tests/parser_tests.py 100% <100%> (ø) ⬆️
arrow/parser.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 43ffd2a...e58f0dc. Read the comment docs.

@systemcatch
Copy link
Collaborator

Hey @dyerd thanks for the contribution. I'm just testing this out and making sure there aren't snags in implementing this.

@systemcatch systemcatch self-requested a review May 1, 2019 13:42
@systemcatch
Copy link
Collaborator

systemcatch commented May 1, 2019

arrow.get('12 October 2018', 'DD MMMM YY') => ParserError ✔️

arrow.get('01 January 123456789101112', 'DD MMMM YYYY') => ParserError ✔️

arrow.get("15/01/2019", ["D/M/YY","D/M/YYYY"]) => <Arrow [2019-01-15T00:00:00+00:00]> ✔️

arrow.get("15/01/19", "D/M/YY") => <Arrow [2019-01-15T00:00:00+00:00]> ✔️

arrow.get('12 October 2018 -0700', 'DD MMMM YYYY Z') => <Arrow [2018-10-12T00:00:00-07:00]> ✔️

arrow.get("68096653015/01/19", "YY/M/DD") => <Arrow [2015-01-19T00:00:00+00:00]>

Seems likes it will still wrongly match if YY is at the start of the string.

@jadchaar
Copy link
Member

jadchaar commented Jul 8, 2019

Closing this PR as this bug has been fully resolved in the Version-0.15.0 branch.

@jadchaar jadchaar closed this Jul 8, 2019
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