Error message causes internal error

[schanzer]

Open https://code.pyret.org/editor#share=1CUza8tsxDscC1YUgp3FPM-52nAkLzaPy&v=5f98106 Evaluate modes(movies-table, "studio")

[sorawee]

Minimal program that causes the error:

builtins.raw-array-sort-nums([raw-array: "a", "b"], false)

[blerner]

Modes currently only works on numeric columns, as evidenced by that call to raw-array-sort-nums at https://github.com/brownplt/pyret-lang/blob/horizon/src/arr/trove/statistics.arr#L56. Making that polymorphic over sortable values should be doable, I think, but it will be slower...

[schanzer]

I don't need it to work on other types - throwing an error would be fine!

[blerner]

I'm not sure how to best give an error here; the ergonomics all are lousy.

• We could add a loop to check that every item in the list is actually a number, and throw an error if it's not. That'll be an unstyled error message (like div-by-0) that doesn't give a good backtrace
• We could add a check in builtins.raw-array-sort-nums, as an ultimate backstop, but that's also a bad idea; you shouldn't be using that function if you aren't careful what you're doing
• We could try to copy-paste the approach taken by the plot library, which fakes up an "eager list checker" annotation that does walk the whole list, and gives a normal-looking annotation failure, but (a) the failure is deeper within the internals of the statistics library, and (b) I don't much want to copy-paste that code from CPO to pyret-lang repos, since it's somewhat hacky anyway

• You could fix it yourself, in the starter file ;-) You could mimic the raise on line 198, and complain to the student that the column doesn't contain quantitative data. You could also try a refinement annotation approach, via

  is-all-numbers = _.all(is-number)
  
  fun ensure-numbers(l :: List<Number>%(is-all-numbers)): l end
  
  # modes  :: (t :: Table, col :: String) -> List<Number>
  fun my-modes( t, col): S.modes( ensure-numbers(t.column(col))) end

  produces It's not perfect, since it reveals List<Number>%(is-all-numbers) in the trace (which is a stupid-looking annotation -- isn't it redundant? -- which is why the plot library uses a hack here...), but it avoids your internal error.

I think the last option (either the raise or refinement version) is the fastest fix for you (and you should fix it for everything in that background code that uses t.column, so mean, median, modes, stdev, r-value etc.).

[schanzer]

@blerner thanks - it's really cool to see how refinements work!

Given that Pyret doesn't allow mixed-type columns, couldn't I just check the first value in the column to determine the type of the rest of the values?

[blerner]

I think that's likely risky: iirc, Pyret guesses what each column ought to contain, but I don't recall the heuristics it uses to do so. Checking all the values in a column is still pretty cheap, at the size of tables you're dealing with, so I'd probably just check them all.

[schanzer]

@blerner that's helpful, as I don't have a sense for what's going to hold up CPO. Done - thank you!