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

timeParse doesn't parse days of week. #29

Closed
obosob opened this issue Jul 5, 2017 · 6 comments
Closed

timeParse doesn't parse days of week. #29

obosob opened this issue Jul 5, 2017 · 6 comments

Comments

@obosob
Copy link

obosob commented Jul 5, 2017

Parsing appears to entirely ignore the day of week.

d3.timeFormat("%a")(d3.timeParse("%a")("Wed"))        // "Mon"
d3.timeFormat("%A")(d3.timeParse("%A")("Wednesday"))  // "Monday"
d3.timeFormat("%w")(d3.timeParse("%w")("3"))          // "1"

It's interesting to note that the tests for this include the full date so pass by virtue of the dates being parsed falling on those days and not because the day-of-week is being correctly parsed.

tape("timeParse(\"%a %m/%d/%Y\")(date) parses abbreviated weekday and date", function(test) {
  var p = timeFormat.timeParse("%a %m/%d/%Y");
  test.deepEqual(p("Sun 01/01/1990"), date.local(1990, 0, 1));
  test.deepEqual(p("Wed 02/03/1991"), date.local(1991, 1, 3));
  test.equal(p("XXX 03/10/2010"), null);
  test.end();
});

It does appear to work in conjunction with %W or %U.

@mbostock
Copy link
Member

mbostock commented Jul 5, 2017

It’s not ignored entirely; it affects whether the input is considered a valid date. However it only determines the date when used in conjunction with a directive that specifies the week (%W or %U); see locale.js.

For example:

d3.timeParse("%a")("Wed") // 1900-01-01
d3.timeParse("%a")("Foo") // null

The reason that %W or %U is required is that unless you specify the exact week, specifying the weekday number is either not enough information to specify the date, or it is redundant (or conflicting) with the other fields. For example, “Monday, January 1990” could be January 1, 8, 15, 22, or 29. And likewise “Monday, January 1, 1990” is always 1990-01-01 and so knowing the weekday is unnecessary to specify the date.

I suppose two enhancements are possible:

  1. The parser could return null if the specified weekday does not match the implied weekday by the other fields. So, “Sunday, January 1, 1990” would return null.

  2. The parser could return a valid date when the date is not completely specified. For example, “Monday, January 1990” could return 1990-01-01. I’m not sure adding this is a good idea without explicitly defining the behavior, and even then, it doesn’t seem especially useful.

If you feel strongly about one or both of these enhancements, we can file a feature request accordingly.

@mbostock mbostock closed this as completed Jul 5, 2017
@obosob
Copy link
Author

obosob commented Jul 6, 2017

Yes, I noticed it is used to validate that it is a valid weekday name.

I do understand that it is ambiguous unless a specific week is referenced, though in keeping with the rest of the API, when there is ambiguity return the first possible match or, better, that day of the week in the week (as specified by %W or %U) that the rest of the parsed string returns to keep chronology (the default start-of-week to use for this should probably be specified by the locale) equivalent to separating the weekday sub string and doing the following:

d3.timeParse("%a %U %H:%M")("Wed "+d3.timeFormat("%U %H:%M")(d3.timeParse("%H:%M")("09:00")))

I also think it would be right to return null if the combination of Weekday and precise date were impossible (enhancement 1).

If I specify "March, 1992" I get 1992-03-01, for example.

Yes, the behavior would want to be documented so that there are no surprises, but i would point out that the current behavior is not specified; the fact that the weekday only affects the returned date if a specific week is referenced; and felt, for me at least, intuitive.

If you remain unconvinced I'll give a specific use-case, what I was trying to do when I noticed the behavior was not as I expected. I am trying to take an arbitrary format temporal axis from a user-provided data file, and have the user specify the format that their "Time" field is to be interpreted as, such that I can use the time in linear scales and perform numeric sorting and such over that domain. The specific data I am working with that is driving this development refers to measured and projected data about a "typical week" given as a Weekday and a Time. For these purposes all I want is the parser to return a Monday or a Tuesday and never care to the granularity of specific dates or seconds to the degree that I'd be unaffected by leap days or leap seconds.

@mbostock
Copy link
Member

mbostock commented Jul 7, 2017

I don’t think we want to do the earliest match on ambiguous dates where only the weekday is specified (enhancement 2 above). The problem is the general case. For example, if you try to parse a date like "Tuesday, January 1” without a year, should it search for the earliest year after 1900 where January 1 is a Tuesday, which would be January 1, 1901? Or should it assume that the year 1900 is implied, and return null because January 1, 1900 is a Monday? Avoiding this search keeps the logic simple: the fields default to January 1, 1900 12:00 AM, and are overridden on parse.

However, I am comfortable with validating the parsed weekday (enhancement 1). Without enhancement 2, the validity of parsing a bare weekday such as “Tuesday” depends on the weekday of the default date January 1, 1900. That’s a Monday, so the desired behavior should be:

var parseDate = d3.timeParse("%a");
parseDate("Mon"); // 1900-01-01
parseDate("Tue"); // null

@obosob
Copy link
Author

obosob commented Jul 12, 2017

I agree with the sentiment on not using the earliest but i think me second suggestion may hold water. In the event a parse format doesn't have an explicit date field (day-of-month or day-of-year), then the a weekday parameter will cause parse to find the first start-of-week valid for the rest of the fields and advance to the next occurrence of day-of-week.

such that (given Sunday-based weeks):

var parseDate = d3.timeParse("%a");
parseDate("Mon"); // 1900-01-08
parseDate("Tue"); // 1900-01-09

i.e. when there is no month it will always be in %U == 01 and follow the same logic when there is, effectively week 1 of Month, not week 0 of Month

and

var parseDate = d3.timeParse("%a %B %Y");
parseDate("Mon March 1991"); // 1991-03-04
parseDate("Tue December 2032"); // 2032-12-07
parseDate("Wed December 2032"); // 2032-12-08

This feels right because it would allow the date to be added to the otherwise identical input string and not return null under enhancement 1.

e.g.

var parseDate = d3.timeParse("%a %B %d %Y");
parseDate("Tue December 07 2032"); // 2032-12-07
parseDate("Wed December 07 2032"); // null

Whereas selecting the Tuesday in the same week as December 01 2032 would result in the 30th of November, which doesn't respect the other fields in the parsed string.

The alternative is to choose the first day of week after the default date given by the rest of the string (so long as there is no day-of-month or day-of-year where a null should still be returned if they don't match) but that would break an expectation of contiguity that I, for one, would have. So i'd say that it should be a constraint that, assuming the locale has a Sunday-based week, for all values of [%a|%A|%w] under any static set of other fields that %U should be the same whilst holding validity with the rest of the fields.

@mbostock
Copy link
Member

Want to take a crack at a pull request? I would like to see the logic.

@obosob
Copy link
Author

obosob commented Jul 18, 2017

Sure, will do when I get some free time. I admit the logic isn't simple, but I think as long as what it does is documented appropriately it would be a valuable feature.

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

No branches or pull requests

2 participants