[Fil]

for flechette, support more date formats.

for apache-arrow: closes #269

Related uwdata/flechette#74

[mbostock]

Is there a way to know from the data.schema.fields whether it will be a bigint so that you don’t have to check every value?

[Fil]

I don't think so; running through (potentially) the whole dataset to check the first non-null value might be more costly (in the worst case, say if you have millions of null values before the first BigInt), than returning this function that checks every value.

[Fil]

Concretely: the diff below lifts the test — however the drawback is that it might have to read the whole dataset. Since we wouldn't in the general case want to format millions of rows, I think the simpler version is probably better?

-  const {unit} = data.schema.fields.find((d) => d.name === column).type;
-  return (value) => formatDate(typeof value === "bigint" ? toMilliseconds(value, unit) : value);
+  for (const value of data.getChild(column)) {
+    if (value == null) continue;
+    if (typeof value !== "bigint") break;
+    const {unit} = data.schema.fields.find((d) => d.name === column).type;
+    return (value) => formatDate(toMilliseconds(value, unit));
+  }
+  return formatDate;

[mbostock]

That’s not what I meant. I meant looking at the schema to avoid checking every value. Checking every value ahead of time is worse than what you were previously doing.

[Fil]

I agree, that's why I didn't push this. There's nothing in the schema that tells us whether we'll get BigInts, and the options are not saved in the file.

[mbostock]

You shouldn’t pass data into the formatTimestamp function here; you should instead fold the unit test into the earlier type check to have separate seconds, milliseconds, microseconds, and nanoseconds types…

Suggested change

	case "timestamp": format[column] = formatTimestamp(data, column); break;
	case "seconds": format[column] = formatSeconds; break;
	case "milliseconds": format[column] = formatMilliseconds; break;
	case "microseconds": format[column] = formatMicroseconds; break;
	case "nanoseconds": format[column] = formatNanoseconds; break;

More generally “timestamp” isn’t sufficiently capturing what the type of this column is, which means the formatter is doing too much work, and is reliant on the data being an Arrow Table. That logic is supposed to live in the type checking routine, not in the formatter.

[mbostock]

Suggested change

	return "timestamp";
	const unit = field.type.unit;
	return unit === 0 ? "seconds" : unit === 1 ? "milliseconds" : unit === 2 ? "microseconds" : unit === 3 ? "nanoseconds" : undefined;