Slightly nicer error messages when a variable is missing in the data table. #235
Conversation
|
This is a great start! It might be slightly nicer to intercept these errors somewhere other than Lines 171 to 174 in 50da19f That also seems like a reasonable place to add this check. |
|
Hi. Sorry for the delay here. I've belatedly had a bash at adding this to the concrete_term constructors. |
| msg = checknamesexist( f, data ) | ||
| if msg != "" | ||
| throw(ArgumentError(msg)) | ||
| end |
There was a problem hiding this comment.
This is now redundant with the concrete_term checks (since those get triggered by the schema call a few lines below). Or, actually, maybe not, since missing_omit may be trying to access the missing columns. Either way I don't think it really does any harm by being here, too, so you get all the missing variables at once.
Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
|
Thanks for this. I'm coming to this Mainly from Ada, hence all the types... |
Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>
|
Ah yeah cool! It's a pretty common part of the julia learning curve to figure out where and how to best use type annotations. A lot of the time they aren't even really necessary in function signatures, since the compiler is pretty good at figuring that stuff out on the fly. The only situations when they're really necessary is when you need special handling for different types of inputs (e.g. for dispatch); otherwise the general advice is to keep the type restrictions on methods as sparse as possible. |
|
Thanks for the contribution! |
A missing variable in a model can produce a very long and confusing error message if the data has many columns. This patch reproduces the nice DataFrames error missing variables error message, and produces only one of the huge messages in the stack trace rather than several (but apart from that isn't as much of an improvement as I'd intended).