Unexpected comparator warning in Vector_Spec

[radeusgd]

I've been getting a following test failure intermittently:

Use a slice of an array as vectors ► should report only a limited number of warnings for incomparable values

(sorted - warnings = [Different comparators: [Standard.Base.Internal.Ordering_Helpers.Default_Comparator], Values NaN and 162 are incomparable, Values 00:00:00 and Date.type.new[Date.enso:103-105] self=Date year=_ are incomparable, Values 429 and NaN are incomparable, Values 319 and 'foo261' are incomparable, Values 'foo261' and 259 are incomparable, Values 242 and 'foo241' are incomparable, Values 00:00:00 and Nothing are incomparable, Values 'foo451' and Nothing are incomparable, Values [] and 392 are incomparable, Values 112 and NaN are incomparable]) 11 did not equal 10 (at C:\runner\_work\enso\enso\test\Base_Tests\src\Data\Vector_Spec.enso:917:13-45).

After adding the with_clue showing the warnings, I think the problem is that we are getting an additional, unexpected warning: Different comparators: [Standard.Base.Internal.Ordering_Helpers.Default_Comparator].

The message itself is weird - it says there are different comparators - but it lists only one comparator (so what is different? I imagine there can be different ones if there is at least 2) and it is the Default_Comparator anyway.

After a clean build, I was no longer able to reproduce this locally, but I was again getting it on CI for #10576

Ideally we should try to understand why this was happening and if a fix is needed.

[JaroslavTulach]

I'll take a look. I guess this is related to my changes in comparators:

• Enforce conversion method return type & redefine Comparable #10468

[JaroslavTulach]

This problem shall be fixed by 7082e49

The 11th warning, which appears there is Different comparators: [Standard.Base.Internal.Ordering_Helpers.Default_Comparator] - there is no point in adding such a warning, when there is only a single comparator.

[radeusgd]

Thanks for the investigation and the fix!

Btw. even if there's more than 1 comparator, isn't the 'Different comparators' warning a bit redundant with the Incomparable_Values warning? What if we could specify what comparators were used as part of the Incomparable_Values payload instead?

Also, I did not notice that back then, but it looks like we are attaching a raw Text type warning here. It may not have been written anywhere officially, but I think that the agreement was that we do not want to do that - every warning/dataflow error/panic should be some more concrete type - it is a bit equivalent to throwing a raw RuntimeException with a message instead of using a more specialized class - it is rather problematic for users e.g. to catch such a warning/error. Could we get this wrapped into a type?

[JaroslavTulach]

Hello Radek.

Btw. even if there's more than 1 comparator, isn't the 'Different comparators' warning a bit redundant with the Incomparable_Values warning? What if we could specify what comparators were used as part of the Incomparable_Values payload instead?

I don't have any strong opinion here. CCing @Akirathan

Also, I did not notice that back then, but it looks like we are attaching a raw Text type warning here. ... Could we get this wrapped into a type?

I don't have any strong opinion here either. I guess it is in lib teams hands to change the API the way you like. It is on border with engine, but the actual thrown exception shouldn't affect the engine at all.