Set precision correctly on full date time strings.

[stevetweeddale]

Full ISO strings including times currently cause the precision to be set as if only year-month was provided.

The parser is parsing it fine, and the underlying native date object is set correctly:

> edtf.parse('2001-02-03T10:10:10')
{ values: [ 2001, 1, 3, 10, 10, 10 ],
  offset: null,
  type: 'Date',
  level: 0 }
> edtf('2001-02-03T10:10:10').toISOString()
'2001-02-03T10:10:10.000Z'

It's just the precision thats off:

> edtf('2001-02-03T10:10:10').precision
2

This has a knock on effect on a whole bunch of stuff, including truncating what you get back from the values getter, and messes up any calls to next(), previous(), max, toEDTF() etc etc.

The issue seems to be the modulo in the precision setter:

  set precision(value) {
    P.set(this, Number(value) % 4) // This would explain it? 6 % 4 == 2
  }

I don't entirely know what I'm doing here, but this pull request removes it, which allows the precision to be 6 in this case. It then also fixes up the toEDTF() method to handle this new eventuality.

All the existing tests still pass, along with a new one to check the precision gets set right.

Let me know if I've gone about this all wrong!

[inukshuk]

Thanks!

The reason for the modulo there was that if I understand the spec correctly, it's not allowed to have partial times: i.e., 2016-07-07T11:50 is not valid. The parser would not accept this at the moment, so I added the modulo to make sure it would not be possible to generate a string which can't be parsed.

Do you think it would make sense for the parser to accept time strings with reduced precision?

[stevetweeddale]

Yeah, basic dates (level 0) are only allowed to be a date (YYYY, YYYY-MM or YYYY-MM-DD), or a full ISO datetime. Partial times are never allowed as far as I can tell, and I don't think the parser should accept that. I hadn't handled that case because I hadn't considered the case of generating an EDTF string from a javascript object with partial time values.

In that case, how about something like this:

  set precision(value) {
    if (value == 6) {
      P.set(this, Number(value))
    }
    else {
      P.set(this, Number(Math.min(value, 3)))
    }
  }

That way full datetimes will output full datetime strings, and a date object with any kind of partial time will consistently truncate to output YYYY-MM-DD, rather than the current behaviour:

> new edtf.Date([2000,1,1,10]) // Precision 4 % 4 == 0
2000-02-01T10:00:00.000Z
> new edtf.Date([2000,1,1,10,10]) // Precision  5 % 4 == 1
2000
> new edtf.Date([2000,1,1,10,10,10]) // Precision 6 % 4 == 2
2000-02

[inukshuk]

Do we need precision 6 though? I think my intention was to allow only 0 (full), 1 (y), 2 (ym), and 3 (ymd) -- so normalising everything above 3 to 3 in the setter should solve the problem, right? Using modulo was simply a bad choice there.

[stevetweeddale]

Ok, so you'd be suggesting something like:

  set precision(value) {
    P.set(this, Number(Math.min(value, 3)))
  }

That would indeed make the output consistent, which is good. But here's the problem with leaving it at that, as I see it: I'm denormalising EDTF date strings (including full datetimes) to min/max values in a DB. Always truncating to precision 3, like above, I'd get:

> new Date(edtf('2000-01-01T10:10:10').max)
2000-01-01T23:59:59.999Z
> new Date(edtf('2000-01-01T10:10:10').min)
2000-01-01T10:10:10.000Z

The max bound isn't right there. The given data is more precise than that.

However, allow 1,2,3 or 6, and I get:

> new Date(edtf('2000-01-01T10:10:10').min)
2000-01-01T10:10:10.000Z
> new Date(edtf('2000-01-01T10:10:10').max)
2000-01-01T10:10:10.999Z

… which seems correct to me. Does that make sense?

[inukshuk]

You're right, what we should do is normalize everything above 3 to 0 (not 3) as 0 means 'full precision' -- that would give you the correct min/max for time strings, right?

By the way, we seem to be working on something similar, I'm also planning to save EDTF strings into a spatial DB index using their min/max values, in fact, that's the reason why I modeled this way.

[inukshuk]

Thanks!