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

:date_time converter fails to recognize "YYYY-MM-DD HH:MM" #247

Closed
thyresias opened this issue Jun 14, 2022 · 7 comments · Fixed by #248
Closed

:date_time converter fails to recognize "YYYY-MM-DD HH:MM" #247

thyresias opened this issue Jun 14, 2022 · 7 comments · Fixed by #248

Comments

@thyresias
Copy link
Contributor

thyresias commented Jun 14, 2022

require "csv"
csv = '2022-06-14 18:48'
p CSV.parse(csv, converters: :date_time).first.first #=> "2022-06-14 18:48"
p DateTime.parse(csv) #=> #<DateTime: 2022-06-14T18:48:00+00:00 ((2459745j,67680s,0n),+0s,2299161j)>

There are two ways to solve this, I'm not sure which is the best one:

  • make the seconds optional:
    DateTimeMatcher =
      / \A(?: (\w+,?\s+)?\w+\s+\d{1,2}\s+\d{1,2}:\d{1,2}:\d{1,2},?\s+\d{2,4} |
              \d{4}-\d{2}-\d{2}\s\d{2}:\d{2}(?::\d{2})? |
              # ISO-8601
              \d{4}-\d{2}-\d{2}
                (?:T\d{2}:\d{2}(?::\d{2}(?:\.\d+)?(?:[+-]\d{2}(?::\d{2})|Z)?)?)?
          )\z /x
  • or allow "T" to be replaced by " ":
    DateTimeMatcher =
      / \A(?: (\w+,?\s+)?\w+\s+\d{1,2}\s+\d{1,2}:\d{1,2}:\d{1,2},?\s+\d{2,4} |
              \d{4}-\d{2}-\d{2}\s\d{2}:\d{2}:\d{2} |
              # ISO-8601
              \d{4}-\d{2}-\d{2}
                (?:[T ]\d{2}:\d{2}(?::\d{2}(?:\.\d+)?(?:[+-]\d{2}(?::\d{2})|Z)?)?)?
          )\z /x
@kou
Copy link
Member

kou commented Jun 14, 2022

The former is the best because ISO 8601 requires T.

@kou
Copy link
Member

kou commented Jun 14, 2022

Do you want to open a pull request for this?

@thyresias
Copy link
Contributor Author

I can sure open a pull request, if you explain to me how this has to be done (or point me to a "how to"): I never did it before.
ありがとう

@kou
Copy link
Member

kou commented Jun 14, 2022

@ericgpks Could you support @thyresias ?

@ericgpks
Copy link
Contributor

of course !

@ericgpks
Copy link
Contributor

@thyresias
Please fork this repository first of all, then create a branch to make a pull request.
You make some changes on that branch and then push and open as a pull request.
If you have any questions, you can ask anytime!!

@thyresias
Copy link
Contributor Author

@ericgpks
Thank you Eriko-san. I hope I did it right.

@olleolleolle olleolleolle linked a pull request Jun 15, 2022 that will close this issue
@kou kou closed this as completed in #248 Jun 29, 2022
kou pushed a commit that referenced this issue Jun 29, 2022
fix #247

ISO 8601 uses "T" for separator of date and time.
RFC 3339 also accepts a space for separator of date and time.
peterzhu2118 pushed a commit to Shopify/ruby that referenced this issue Dec 6, 2022
(ruby/csv#248)

fix ruby/csv#247

ISO 8601 uses "T" for separator of date and time.
RFC 3339 also accepts a space for separator of date and time.

ruby/csv@1255d8c304
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants