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

Resolves #114 dates are validated for short months and leap years #158

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

UnicodingUnicorn
Copy link
Contributor

Overview

What it says on the tin. Resolves #114. What was happening was that moment.js actually catches and throws errors on such things just fine, just that d3-time-format changes the supplied date to the next closest valid one, creating the error as described.

I changed it so that default uses a moment.js format string to parse the date, rather than using the timeParse of d3-time-format. fmt remains the same. I also chucked in a couple of tests for short months and leap dates.


Please preserve this line to notify @roll (lead of this repository)

@roll
Copy link
Member

roll commented Oct 28, 2019

@UnicodingUnicorn
Thanks but I think the problem is still here because we use timeParse for custom formats. And we rely on these format strings not compatible with moment.js

Any chance we can make timeParse raise these errors?

@roll roll added the review label Oct 28, 2019
@UnicodingUnicorn
Copy link
Contributor Author

@roll I believe that this problem is mentioned in d3/d3-time-format#47. I don't think timeParse can, or will, raise an error now or in the future.

As an aside, may I ask why the decision to use an additional library? As far as I can see, moment.js time parsing offers similar functionality, albeit with different syntax.

@roll
Copy link
Member

roll commented Oct 30, 2019

@UnicodingUnicorn
Thanks for the information. Datetime objects in JavaScript is a joke... Yea, I don't see how it can be resolved for custom datetime-formats.

Regarding the syntax - this library is an implementation of the Table Schema spec along with other 8 implementations. So on the lib level, we don't choose what syntax to use. And the reason is interoperability between different platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

date validation doesn't check for leap years or short months
2 participants