InseeFr / Trevas

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

Sqrt #341

Open NicoLaval opened 1 month ago

NicoLaval commented 1 month ago

Sqrt throws errors for calls with null / NaN / negative values, BdI tests assume null to be the correct result.

NicoLaval commented 1 month ago

@noahboerger

id a
IDENTIFIER MEASURE
INTEGER INTEGER
1 2
2 -2
3 null

Trevas ds_1 result will be:

id a sq
IDENTIFIER MEASURE MEASURE
INTEGER INTEGER INTEGER
1 2 4
2 -2 4
3 null null

Thanks to dataset metadata, null is recognized as integer.

NicoLaval commented 1 month ago

TODO: fix behaviour for negative inputs

noahboerger commented 1 month ago

Thank you for the explanations and plan to fix it for negative inputs.

hadrienk commented 1 month ago

Just a comment here, but I think I've seen somewhere in the spec that type inference should be supported. It's a very small paragraph and there's not much details. Technically I think this isn't too hard to implement and has great value for the users. But this need to be reworked in the spec.

NicoLaval commented 1 month ago

Not sure about that. What will be rewrited in 2.1 is implicit casting between known types.

I think your right, when there is no doubt about the null type, we can apply an implicit cast, avoiding users to write lot of cast.

I opened an issue in the TF repo