review

Author: GregoryTravis

distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Add_Group_Number.enso

@@ -1,3 +1,5 @@
+private
+
 from Standard.Base import all
 import Standard.Base.Errors.Common.Unsupported_Argument_Types
 import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
@@ -28,7 +30,7 @@ add_group_number (table:Table) (grouping_method:Grouping_Method) (name:Text) (fr
                         grouping = _prepare_group_by table problem_builder group_by
                         AddGroupNumber.numberGroupsUnique table.row_count from step grouping java_problem_aggregator
                 Grouping_Method.Equal_Count group_count order_by ->
-                    _illegal_if (group_count==0) "group_count must be at least 1" <|
+                    _illegal_if (group_count < 1) "group_count must be at least 1" <|
                         ordering = _prepare_ordering table problem_builder order_by
                         AddGroupNumber.numberGroupsEqualCount table.row_count group_count from step (ordering.at 0) (ordering.at 1) java_problem_aggregator
             new_column = Column.from_storage name new_storage

std-bits/table/src/main/java/org/enso/table/operations/AddGroupNumber.java

@@ -30,7 +30,7 @@ public static Storage<?> numberGroupsUnique(
 
     var groupNumberIterator = new StepIterator(start, step);
 
-    long[] numbers = new long[(int) numRows];
+    long[] numbers = new long[Math.toIntExact(numRows)];
 
     Storage<?>[] groupingStorages =
         Arrays.stream(groupingColumns).map(Column::getStorage).toArray(Storage[]::new);
@@ -59,7 +59,7 @@ public static Storage<?> numberGroupsEqualCount(
       Column[] orderingColumns,
       int[] directions,
       ProblemAggregator problemAggregator) {
-    long[] numbers = new long[(int) numRows];
+    long[] numbers = new long[Math.toIntExact(numRows)];
 
     var equalCountGenerator = new EqualCountGenerator(start, step, numRows, groupCount);
 
@@ -72,7 +72,7 @@ public static Storage<?> numberGroupsEqualCount(
           Arrays.stream(orderingColumns).map(Column::getStorage).toArray(Storage[]::new);
       List<OrderedMultiValueKey> keys =
           new ArrayList<>(
-              IntStream.range(0, (int) numRows)
+              IntStream.range(0, Math.toIntExact(numRows))
                   .mapToObj(i -> new OrderedMultiValueKey(orderingStorages, i, directions))
                   .toList());
       keys.sort(null);
@@ -104,7 +104,7 @@ public long next() {
   private static class EqualCountGenerator {
     private final long start;
     private final long step;
-    private long current = 0;
+    private long currentIndex = 0;
     private final long groupSize;
 
     public EqualCountGenerator(long start, long step, long totalCount, long numgroups) {
@@ -114,8 +114,8 @@ public EqualCountGenerator(long start, long step, long totalCount, long numgroup
     }
 
     public long next() {
-      long toReturn = Math.addExact(start, Math.multiplyExact(step, (current / groupSize)));
-      current = Math.addExact(current, 1L);
+      long toReturn = Math.addExact(start, Math.multiplyExact(step, (currentIndex / groupSize)));
+      currentIndex = Math.addExact(currentIndex, 1L);
       return toReturn;
     }
   }

test/Table_Tests/src/Common_Table_Operations/Add_Group_Number_Spec.enso

@@ -74,6 +74,16 @@ add_group_number_specs suite_builder setup =
             g7 = t.add_group_number (..Equal_Count 2 order_by='z') "g"
             g7.at 'g' . to_vector . should_equal [0, 1, 1, 0, 0]
 
+            g8 = t.add_group_number (..Equal_Count 1) "g"
+            g8.at 'g' . to_vector . should_equal [0, 0, 0, 0, 0]
+
+            g9 = t.add_group_number (..Equal_Count 1 order_by=['x']) "g"
+            g9.at 'g' . to_vector . should_equal [0, 0, 0, 0, 0]
+
+            g10 = t.add_group_number (..Equal_Count 1 order_by=['y']) "g"
+            g10.at 'g' . to_vector . should_equal [0, 0, 0, 0, 0]
+
+
         group_builder.specify "should add group number by unique values" <|
             t = table_builder_from_rows ['x', 'y', 'z'] [[1, 0, 2], [0, 1, 0], [1, 2, 0], [0, 1, 1], [1, 0, 1], [1, 2, 1]]
 
@@ -97,6 +107,16 @@ add_group_number_specs suite_builder setup =
 
             t.add_group_number ..Unique "g" . should_fail_with Missing_Argument
 
+        group_builder.specify "must specify nonempty group_by with Unique" <|
+            t = table_builder_from_rows ['x', 'y', 'z'] [[1, 0, 2], [0, 1, 0], [1, 2, 0], [0, 1, 1], [1, 0, 1], [1, 2, 1]]
+
+            t.add_group_number (..Unique on=[]) "g" . should_fail_with Illegal_Argument
+
+        group_builder.specify "must specify one or more groups with Equal_Count" <|
+            t = table_builder [['x', [1, 2, 3, 4, 5]], ['y', [5, 4, 3, 2, 1]], ['z', [1, 5, 4, 2, 3]]]
+            t.add_group_number (..Equal_Count 0) "g" . should_fail_with Illegal_Argument
+            t.add_group_number (..Equal_Count -1) "g" . should_fail_with Illegal_Argument
+
         group_builder.specify "should report floating point equality warning when grouping on float columns" <|
             t = table_builder_from_rows ['x', 'y', 'z'] [[1.0, 0.0, 2.0], [0.0, 1.0, 0.0], [1.0, 2.0, 0.0], [0.0, 1.0, 1.0], [1.0, 0.0, 1.0], [1.0, 2.0, 1.0]]
             g0 = t.add_group_number (..Unique on=['x', 'y']) "g"
@@ -124,8 +144,9 @@ add_group_number_specs suite_builder setup =
             t2.catch.to_display_text . should_contain "The row number has exceeded the 64-bit integer range"
 
         group_builder.specify "should rename existing column upon a name clash, and attach a warning" <|
-            t = table_builder_from_rows ['x', 'y', 'z'] [[1, 0, 2], [0, 1, 0], [1, 2, 0], [0, 1, 1], [1, 0, 1], [1, 2, 1]]
+            t = table_builder_from_rows ['x', 'y', 'z'] [['b', 'a', 'c'], ['a', 'b', 'a'], ['b', 'b', 'a'], ['a', 'b', 'b'], ['b', 'a', 'b'], ['b', 'b', 'b']]
 
             g0 = t.add_group_number (..Unique on=['x', 'y']) "y"
-            g0.at 'y 1' . to_vector . should_equal [0, 1, 2, 1, 0, 2]
+            g0.at 'y' . to_vector . should_equal [0, 1, 2, 1, 0, 2]
+            g0.at 'y 1' . to_vector . should_equal ['a', 'b', 'b', 'b', 'a', 'b']
             Problems.expect_warning Duplicate_Output_Column_Names g0