From 8949f20e4743cae3da7d825582f4cde762331d06 Mon Sep 17 00:00:00 2001 From: Daniel Patterson Date: Fri, 27 Jun 2025 12:22:44 -0400 Subject: [PATCH] Change table load to retain original header row, for improved errors --- src/arr/trove/csv.arr | 20 ++++++++++++----- src/arr/trove/error.arr | 36 +++++++++++++++++++++++++++++++ src/js/trove/ffi.js | 6 ++++++ src/js/trove/table.js | 10 +++++---- tests/pyret/tests/test-tables.arr | 2 +- 5 files changed, 64 insertions(+), 10 deletions(-) diff --git a/src/arr/trove/csv.arr b/src/arr/trove/csv.arr index 52ddbe62b..9051d666a 100644 --- a/src/arr/trove/csv.arr +++ b/src/arr/trove/csv.arr @@ -60,7 +60,7 @@ fun to-content(csv :: RawArray>) end -fun csv-table-opt(csv :: RawArray>, opts :: CSVOptions): +fun csv-table-opt(csv :: RawArray>, orig-headers :: O.Option>, opts :: CSVOptions): shadow opts = builtins.record-concat(default-options, opts) { load: lam(headers, sanis) block: @@ -76,20 +76,30 @@ fun csv-table-opt(csv :: RawArray>, opts :: CSVOptions): result end - { headers; contents } + { headers; orig-headers; contents } end } end fun csv-table(csv :: RawArray>) -> { load :: Function }: - csv-table-opt(csv, default-options) + csv-table-opt(csv, O.none, default-options) end fun csv-table-str(csv :: String, opts): shadow opts = builtins.record-concat(default-options, opts) skip-rows = if opts.header-row: 1 else: 0 end - rows = csv-lib.parse-string(csv, { skipRows: skip-rows }) - csv-table-opt(rows, opts) + rows = csv-lib.parse-string(csv, {}) + contents = if opts.header-row: + raw-array-from-list(raw-array-to-list(rows).drop(1)) + else: + rows + end + headers = if opts.header-row and (raw-array-length(rows) > 0): + O.some(raw-array-get(rows, 0)) + else: + O.none + end + csv-table-opt(contents, headers, opts) end fun csv-table-file(path :: String, opts): diff --git a/src/arr/trove/error.arr b/src/arr/trove/error.arr index fb649d860..7299f7a00 100644 --- a/src/arr/trove/error.arr +++ b/src/arr/trove/error.arr @@ -18,6 +18,9 @@ end fun horz-list-values(vals): [ED.para: ED.h-sequence(vals.map(lam(val): ED.embed(val) end), ",") ] end +fun horz-list-str-ids(vals): + [ED.para: ED.h-sequence(vals.map(lam(val): ED.text(val) end), ",") ] +end fun ed-simple-intro(name, loc): ed-simple-intro-a-an(name, loc, "a") @@ -1925,6 +1928,39 @@ data RuntimeError: vert-list-values(self.fun-app-args)]) end + | header-row-mismatch(expected-headers, orig-headers, provided-row) with: + method render-fancy-reason(self, maybe-stack-loc, src-available, maybe-ast): + cases(O.Option) maybe-stack-loc(0, true): + | some(l) => + [ED.error: + ed-intro("table construction", l, -1, true), + [ED.para: ED.text("The table could not be constructed because the number of expected headers didn't match the number found in the data.")], + [ED.para: ED.text("Expected to have " + num-to-string(raw-array-length(self.expected-headers)) + " columns:")], + horz-list-str-ids(raw-array-to-list(self.expected-headers)), + cases(O.Option) self.orig-headers: + | some(oh) => + [ED.para: [ED.para: ED.text("Instead, found header with " + num-to-string(raw-array-length(oh)) + " columns:"), + horz-list-str-ids(raw-array-to-list(oh))], + [ED.para: ED.text("And data with " + num-to-string(raw-array-length(self.provided-row)) + " columns:"), + horz-list-values(raw-array-to-list(self.provided-row))]] + | none => + [ED.para: ED.text("But found data with " + num-to-string(raw-array-length(self.provided-row)) + " columns:"), + horz-list-values(raw-array-to-list(self.provided-row))] + end, + ] + | none => self.render-reason() + end + end, + method render-reason(self): + [ED.error: + [ED.para: ED.text("The row could not be constructed because the number of columns didn't match the number of provided values.")], + [ED.para: ED.text("Expected " + num-to-string(raw-array-length(self.colnames)) + " columns:")], + ED.embed(self.colnames), + [ED.para: ED.text("Provided " + num-to-string(raw-array-length(self.provided-vals)) + " values:")], + horz-list-values(raw-array-to-list(self.provided-vals))] + end + + | row-length-mismatch(colnames, provided-vals) with: method render-fancy-reason(self, maybe-stack-loc, src-available, maybe-ast): cases(O.Option) maybe-stack-loc(0, true): diff --git a/src/js/trove/ffi.js b/src/js/trove/ffi.js index f6aacac34..034c20427 100644 --- a/src/js/trove/ffi.js +++ b/src/js/trove/ffi.js @@ -371,6 +371,11 @@ raise(err("arity-mismatch")(funLoc, arity, args, isMethod)); } + function throwHeaderRowMismatch(colnames, origHeaders, providedVals) { + runtime.checkArray(providedVals); + raise(err("header-row-mismatch")(colnames, origHeaders, providedVals)); + } + function throwRowLengthMismatch(colnames, providedVals) { runtime.checkArray(providedVals); raise(err("row-length-mismatch")(colnames, providedVals)); @@ -634,6 +639,7 @@ throwUninitializedIdMkLoc: throwUninitializedIdMkLoc, throwArityError: throwArityError, throwArityErrorC: throwArityErrorC, + throwHeaderRowMismatch: throwHeaderRowMismatch, throwRowLengthMismatch: throwRowLengthMismatch, throwColLengthMismatch: throwColLengthMismatch, throwConstructorArityErrorC: throwConstructorArityErrorC, diff --git a/src/js/trove/table.js b/src/js/trove/table.js index 83937424e..f6caee314 100644 --- a/src/js/trove/table.js +++ b/src/js/trove/table.js @@ -165,14 +165,16 @@ function openTable(info) { runtime.checkTuple(info); - if (info.vals.length != 2) { - runtime.ffi.throwMessageException("Expected to find {header; contents} pair, " + if (info.vals.length != 3) { + runtime.ffi.throwMessageException("Expected to find {header; orig-headers; contents} triple, " + "but found a tuple of length " + info.vals.length); } var headers = info.vals[0]; - var contents = info.vals[1]; + var origHeaders = info.vals[1]; + var contents = info.vals[2]; runtime.checkArray(headers); + // runtime.checkPyretVal(origHeaders); // Can we do better? runtime.checkArray(contents); var names = []; var sanitizers = []; @@ -194,7 +196,7 @@ runtime.checkArray(contents[i]); if (contents[i].length !== headers.length) { if (i === 0) { - throw runtime.ffi.throwRowLengthMismatch(names, contents[i]); + throw runtime.ffi.throwHeaderRowMismatch(names, origHeaders, contents[i]); } else { throw runtime.ffi.throwMessageException("Contents must be rectangular"); } diff --git a/tests/pyret/tests/test-tables.arr b/tests/pyret/tests/test-tables.arr index 5f5fc52f3..46fdb4080 100644 --- a/tests/pyret/tests/test-tables.arr +++ b/tests/pyret/tests/test-tables.arr @@ -90,7 +90,7 @@ check "Basic Table Loading": {raw-array-get(titles, 1); sanitizer.sanitizer}] contents = [raw-array: [raw-array: DS.c-str(name), DS.c-str("12")], [raw-array: DS.c-str("Alice"), DS.c-num(15)]] - {headers; contents} + {headers; none; contents} end make-loader = {(x): {load: lam(titles, sanitizer): with-tl(x, titles, sanitizer) end}} (load-table: name, age