InseeFr / Trevas

Transformation engine and validator for statistics.
MIT License
12 stars 5 forks source link

If then else #323

Open NicoLaval opened 6 months ago

NicoLaval commented 6 months ago

@noahboerger you reported If-then-else is not supported.

I thought it was, for scalars, components and datasets.

Could you report a failed example please?

noahboerger commented 6 months ago

The example i have tested is from the BdI testcases is the one under "11. conditional/if-then-else_2".

The following statement has been failing: result:= d1[calc Me_2 := if (Id_4 = "F") then Me_1/1000 else 0];

Occured error ### Exception ``` fr.insee.vtl.engine.exceptions.FunctionNotFoundException: function 'ifThenElse(Boolean, Double, Long)' not found at fr.insee.vtl.engine.visitors.expression.functions.GenericFunctionsVisitor.invokeFunction(GenericFunctionsVisitor.java:146) at fr.insee.vtl.engine.visitors.expression.ConditionalVisitor.visitIfExpr(ConditionalVisitor.java:100) at fr.insee.vtl.engine.visitors.expression.ConditionalVisitor.visitIfExpr(ConditionalVisitor.java:20) at fr.insee.vtl.parser.VtlParser$IfExprContext.accept(VtlParser.java:657) at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18) at fr.insee.vtl.engine.visitors.expression.ExpressionVisitor.visitIfExpr(ExpressionVisitor.java:226) at fr.insee.vtl.engine.visitors.expression.ExpressionVisitor.visitIfExpr(ExpressionVisitor.java:41) at fr.insee.vtl.parser.VtlParser$IfExprContext.accept(VtlParser.java:657) at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visitChildren(AbstractParseTreeVisitor.java:46) at fr.insee.vtl.parser.VtlBaseVisitor.visitCalcClauseItem(VtlBaseVisitor.java:594) at fr.insee.vtl.parser.VtlParser$CalcClauseItemContext.accept(VtlParser.java:5686) at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18) at fr.insee.vtl.engine.visitors.ClauseVisitor.visitCalcClause(ClauseVisitor.java:149) at fr.insee.vtl.engine.visitors.ClauseVisitor.visitCalcClause(ClauseVisitor.java:33) at fr.insee.vtl.parser.VtlParser$CalcClauseContext.accept(VtlParser.java:1797) at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visitChildren(AbstractParseTreeVisitor.java:46) at fr.insee.vtl.parser.VtlBaseVisitor.visitDatasetClause(VtlBaseVisitor.java:230) at fr.insee.vtl.parser.VtlParser$DatasetClauseContext.accept(VtlParser.java:1494) at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18) at fr.insee.vtl.engine.visitors.expression.ExpressionVisitor.visitClauseExpr(ExpressionVisitor.java:357) at fr.insee.vtl.engine.visitors.expression.ExpressionVisitor.visitClauseExpr(ExpressionVisitor.java:41) at fr.insee.vtl.parser.VtlParser$ClauseExprContext.accept(VtlParser.java:683) at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18) at fr.insee.vtl.engine.visitors.AssignmentVisitor.visitAssignment(AssignmentVisitor.java:51) at fr.insee.vtl.engine.visitors.AssignmentVisitor.visitTemporaryAssignment(AssignmentVisitor.java:59) at fr.insee.vtl.parser.VtlParser$TemporaryAssignmentContext.accept(VtlParser.java:372) at org.antlr.v4.runtime.tree.AbstractParseTreeVisitor.visit(AbstractParseTreeVisitor.java:18) at fr.insee.vtl.engine.VtlScriptEngine.evalStream(VtlScriptEngine.java:263) at fr.insee.vtl.engine.VtlScriptEngine.eval(VtlScriptEngine.java:282) at java.scripting/javax.script.AbstractScriptEngine.eval(AbstractScriptEngine.java:262) at fr.insee.trevas.jupyter.VtlKernel.eval(VtlKernel.java:305) at io.github.spencerpark.jupyter.kernel.BaseKernel.handleExecuteRequest(BaseKernel.java:334) at io.github.spencerpark.jupyter.channels.ShellChannel.lambda$bind$0(ShellChannel.java:64) at io.github.spencerpark.jupyter.channels.Loop.lambda$new$0(Loop.java:21) at io.github.spencerpark.jupyter.channels.Loop.run(Loop.java:78) ```
NicoLaval commented 6 months ago

Bug approved. Thanks.

hadrienk commented 6 months ago

Isn't it a type problem? I think this should work:

result:= d1[calc Me_2 := if (Id_4 = "F") then Me_1/1000 else 0.0]; (note the 0.0 instead of 0)

The error message could be better, the function not found is misleading in this case.

noahboerger commented 6 months ago

@hadrienk you are right, thank you. With the 0.0 it is working.

NicoLaval commented 6 months ago

Sure it works but as we have done in ComparisonVisitor, we have to support this mixt of number and integer, instead of asking to the user to write a tricky 0.0

noahboerger commented 5 months ago

Would be nice if also a mix of other data types e.g. with null is supported.

result:= d1[calc Me_2 := if (Id_4 = "F") then Me_1/1000 else null]; is also quite useful out of our point of view.

NicoLaval commented 5 months ago

What's about:


result:= d1[calc Me_2 := if (Id_4 = "F") then Me_1/1000 else cast(null, number)];

?

I think the actual specification doesn't define null type inference, maybe it could be defined and then handled.

NicoLaval commented 5 months ago

I opened an issue on the Task Force repo

noahboerger commented 5 months ago

Thank you!