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

allow %d and %m to take either 1 or 2 characters, e.g. 03/04/1999 or 3/4/1999 #160

Merged
merged 5 commits into from Sep 25, 2019

Conversation

edzer
Copy link
Contributor

@edzer edzer commented Jul 8, 2019

The problem seems that dates like 1/5/1999 are not read with
format "%m/%d/%Y", because 01/05/1999 is expected. I think
that if you read ?strptime well, this is correct, but then base
R works differently and accepts single digit days and months. So,
I think the prefered behavior is to accept them.

This PR is messy and full with print statements. I believe that the
line marked with "need to set back the parser iterator/pointer one
character here?" should be replaced with setting back the parser
pointer one step, as we have a fail read here which does NOT raise
an error. The error then happens in next step, when the "/" (or other
separator) has been absorbed, I suspect. Happy to revisit this when
I know how the parser works. Please ignore otherwise.

edzer added 2 commits July 8, 2019 14:31
The problem seems that dates like 1/5/1999 are not read with
format "%m/%d/%Y", because 01/05/1999 is expected. I think
that if you read ?strptime well, this is correct, but then base
R works differently and accepts single digit days and months. So,
I think the prefered behavior is to accept them.

This PR is messy and full with print statements. I believe that the
line marked with "need to set back the parser iterator/pointer one
character here?" should be replaced with setting back the parser
pointer one step, as we have a fail read here which does NOT raise
an error. The error then happens in next step, when the "/" (or other
separator) has been absorbed, I suspect. Happy to revisit this when
I know how the parser works.
@edzer
Copy link
Contributor Author

edzer commented Jul 8, 2019

I think I fixed it, now.

@edzer edzer changed the title First shot, but still no solution to #123 allow %d and %m to take either 1 or 2 characters, e.g. 03/04/1999 or 3/4/1999 Jul 8, 2019
@edzer
Copy link
Contributor Author

edzer commented Sep 24, 2019

Hi Jim, any news on this one?

@jimhester
Copy link
Collaborator

Sorry, I did not see that you had resolved this and had been working on other packages since July, thank you for pinging me, if you could
please add a bullet to NEWS? It should briefly describe the change and end with (@yourname, #issuenumber), then I can merge this. Thank you for working on it!

@jimhester jimhester merged commit 9625ca7 into tidyverse:master Sep 25, 2019
@jimhester
Copy link
Collaborator

Thanks for working on this Edzer!

Great work, this package is now better thanks to you!

Thanks!! You are da bomb!

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