-
Notifications
You must be signed in to change notification settings - Fork 289
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
Add DateTimeOffset for Csv Schema Inference #1304
Add DateTimeOffset for Csv Schema Inference #1304
Conversation
The changes are in lines 22, 32, and 41. The rest is whitespace edits to keep formatting consistent with the prior style of lining everything up vertically.
What's required to get this merged? |
The maintainers probably want to chime in on the Date schema API. The default was changed date for the provider was changed to DateTimeOffset in the 3.X move, but none of the schema code was updated to correspond. An alternative to my changes here would be to change the old code so that 'date' implies DateTimeOffset rather than DateTime. I think having the |
The change looks fine, can you add test cases for this? |
Will do.
|
The API fails with the old restriction. It works with a smaller date restriction like 2000:2010, but for simplicity just removing the restriction (which also works and gets the whole history). http://api.worldbank.org/v2/country/gb/indicator/SP.POP.TOTL?format=json&date=
I think this is now ready. Additions:
This set of commits depends on another pull request (#1320), which has changes necessary to get the build working. |
The goal is to allow specifying
The changes are in lines 22, 32, and 41. The rest of the differences are whitespace edits to keep formatting consistent with the prior style of lining everything up vertically.