fix date typing in arrow tables #2096
Conversation
uses the same duck type test as observablehq/inputs#263 closes observablehq/framework#1376
| values = Array.from(values); | ||
| for (const f of fields) { | ||
| if (String(f).endsWith("<MILLISECOND>")) | ||
| values.some((d, i) => d[f.name] != null && (values[i] = {...values[i], [f.name]: new Date(values[i][f.name])})); |
There was a problem hiding this comment.
We already have Date coercion if the scale type is set to temporal. Can we use that instead here? I.e., change the isTemporal test instead of changing arrayify? I think we want to minimize the logic here.
There was a problem hiding this comment.
I don't know if we could do that. As soon as we apply Array.from to the arrow table, the schema information that we need to distinguish between a column of numbers and a column of dates is lost. The data becomes an Array of Proxy objects, with getters that return numbers (even on date-typed columns, per apache/arrow#40892 — note that this change discusses a future improvement to make the Proxy return Date objects again, but I don't think it has happened yet).
But in fact, applying Array.from is already a losing proposition performance-wise, and we should use the columnar format directly (#2030). In a sense, this PR is only a stop-gap measure. I've ported the necessary changes to #2030 too, if we want to take that leap instead.
uses the same duck type test as observablehq/inputs#263
addresses observablehq/framework#1376 (for the Plot part)
Note: it's not as good as full Arrow support (where the data would not get transformed into an array of Proxy objects, and valueof would use table.getChild, see #2030).