Improve error message on Filter_Condition missing arguments in Table.filter
#7290
Conversation
radeusgd
added
the
CI: No changelog needed
| filter self column filter=(Filter_Condition.Equal True) on_problems=Report_Warning = case column of | ||
| filter self column (filter : Filter_Condition | Function = Filter_Condition.Equal True) on_problems=Report_Warning = case column of |
There was a problem hiding this comment.
This has to be Filter_Condition | Function instead of Filter_Condition|(Any->Boolean) due to #7289
| element -> (== element) | ||
|
|
||
| ## PRIVATE | ||
| Checks if the given `function` is actually a `Filter_Condition` constructor | ||
| that is just missing arguments. If so, it will report a more friendly error. | ||
| Otherwise it will just return the `function` as is. | ||
| handle_constructor_missing_arguments function = | ||
| Otherwise it will run the `continuation`. |
There was a problem hiding this comment.
This seems like something we'll need for other constructors, perhaps it should be made generic.
There was a problem hiding this comment.
Agree we may want to make this generic until we can improve type checking. Not one for this PR, but maybe next time we add one.
There was a problem hiding this comment.
The issue is - we are doing a bit of a heuristic to check if this is really a Filter_Condition - by looking at the text representation.
If we want to make it generic, we'd have to make this more robust as that does not seem like a good 'hack' to spread around.
Anyway, this only applies to situations where we have X | Function types. For types that do not accept a Function (most of our ADT args do not), it will be ideal to have this check done at runtime typecheck level.
| element -> (== element) | ||
|
|
||
| ## PRIVATE | ||
| Checks if the given `function` is actually a `Filter_Condition` constructor | ||
| that is just missing arguments. If so, it will report a more friendly error. | ||
| Otherwise it will just return the `function` as is. | ||
| handle_constructor_missing_arguments function = | ||
| Otherwise it will run the `continuation`. |
There was a problem hiding this comment.
Agree we may want to make this generic until we can improve type checking. Not one for this PR, but maybe next time we add one.
Pull Request Description
In #7148 I improved the error message when a
Filter_Conditionconstructor without arguments is provided toVector.filterand its friends. This PR applies the same check to theTable.filter.This is useful, because when we select a Filter_Condition from a widget, initially it does not have all its arguments applied. This used to lead to confusing errors being reported to the user, now, a much clearer error is shown:
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.