make sure filling missing values gets correct type

enso-org · Apr 19, 2023 · ddb7f67 · ddb7f67
1 parent 8d3768e
commit ddb7f67
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 7 deletions.
diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Column.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Column.enso
@@ -812,10 +812,11 @@ type Column
         common_type.if_not_error <|
             new_name = Naming_Helpers.function_name "fill_nothing" [self, default]
             storage = self.java_column.getStorage
+            storage_type = Storage.from_value_type_strict common_type
             new_st = case default of
                 Column.Value java_col ->
                     other_storage = java_col.getStorage
-                    storage.fillMissingFrom other_storage
+                    storage.fillMissingFrom other_storage storage_type
                 _ ->
                     storage.fillMissing default
             col = Java_Column.new new_name new_st

diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Type/Value_Type_Helpers.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Type/Value_Type_Helpers.enso
@@ -162,7 +162,6 @@ type Operation_Type_Helpers
     find_common_type_for_arguments self arguments =
         types = arguments.map self.find_argument_type . filter Filter_Condition.Not_Nothing
         case types.is_empty of
-            # TODO [RW] what to do with `x.iif Nothing Nothing`?
             True -> Nothing
             False -> case find_common_type types strict=True of
                 common_type : Value_Type -> common_type

diff --git a/std-bits/table/src/main/java/org/enso/table/data/column/storage/LongStorage.java b/std-bits/table/src/main/java/org/enso/table/data/column/storage/LongStorage.java
@@ -1,5 +1,6 @@
 package org.enso.table.data.column.storage;
 
+import org.enso.base.polyglot.NumericConverter;
 import org.enso.table.data.column.builder.object.Builder;
 import org.enso.table.data.column.builder.object.NumericBuilder;
 import org.enso.table.data.column.operation.map.MapOpStorage;
@@ -139,7 +140,7 @@ private Storage<?> fillMissingLong(long arg) {
   @Override
   public Storage<?> fillMissing(Value arg) {
     if (arg.isNumber()) {
-      if (arg.fitsInLong()) {
+      if (NumericConverter.isCoercibleToLong(arg.as(Object.class))) {
         return fillMissingLong(arg.asLong());
       } else {
         return fillMissingDouble(arg.asDouble());

diff --git a/std-bits/table/src/main/java/org/enso/table/data/column/storage/Storage.java b/std-bits/table/src/main/java/org/enso/table/data/column/storage/Storage.java
@@ -245,10 +245,11 @@ public Storage<?> fillMissing(Value arg) {
    * Fills missing values in this storage, by using corresponding values from {@code other}.
    *
    * @param other the source of default values
+   * @param commonType a common type that should fit values from both storages
    * @return a new storage with missing values filled
    */
-  public Storage<?> fillMissingFrom(Storage<?> other) {
-    var builder = new InferredBuilder(size());
+  public Storage<?> fillMissingFrom(Storage<?> other, StorageType commonType) {
+    var builder = Builder.getForType(commonType, size());
     for (int i = 0; i < size(); i++) {
       if (isNa(i)) {
         builder.appendNoGrow(other.getItemBoxed(i));

diff --git a/test/Table_Tests/src/Common_Table_Operations/Missing_Values_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Missing_Values_Spec.enso
@@ -200,14 +200,17 @@ spec setup =
             decimals_filled.value_type.is_floating_point.should_be_true
 
         Test.specify "should not allow mixing types by default" <|
-            table = table_builder [["X", [1, Nothing, 2, Nothing]]]
+            table = table_builder [["X", [1, Nothing, 2, Nothing]], ["Y", [True, False, Nothing, Nothing]], ["Z", [0.5, Nothing, Nothing, 0.25]]]
             ints = table.at "X"
             ints_filled = ints.fill_nothing False
-            ints_filled.to_vector . should_fail_with No_Common_Type
+            ints_filled.should_fail_with No_Common_Type
 
             c = ints.coalesce [42.0, False]
             c.to_vector . should_fail_with No_Common_Type
 
+            table.at "Y" . fill_nothing 42 . should_fail_with No_Common_Type
+            table.at "Z" . fill_nothing True . should_fail_with No_Common_Type
+
         if setup.is_database.not then
             Test.specify "may allow mixed types if explicitly retyped" pending="TODO: cast #6112" <|
                 table = table_builder [["X", [1, Nothing, 2, Nothing]]]