-
Notifications
You must be signed in to change notification settings - Fork 323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement Table.add_group_number
, with operations Unique
and Equal_Count
#11818
Changes from all commits
d16cfe7
c01550c
8bc55e7
eedd70a
842d22a
9796878
df8f115
57b7218
c0add8a
056957a
b1a4df5
25a4c2d
03f757c
c8779c4
d278007
c0ec57d
8524232
07c5eca
0777f3e
8441eb0
7ada84c
55a8b5c
19e4a8a
9d86f51
072093f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
from Standard.Base import all | ||
import Standard.Base.Errors.Common.Missing_Argument | ||
|
||
polyglot java import org.enso.table.operations.AddGroupNumber | ||
|
||
## Specifies a method for grouping rows in `add_group_number`. | ||
type Grouping_Method | ||
## Group rows by the specified columns. | ||
|
||
Arguments: | ||
- on: Rows that have the same values for these columns will be grouped | ||
together. At least one column must be specified. | ||
Unique (on:(Vector | Text | Integer | Regex)=(Missing_Argument.throw "on")) | ||
|
||
## Create the specified number of groups with the same number of rows in | ||
each group (except possibly the last one). | ||
|
||
Arguments | ||
- group_count: The number of groups to divide the table into. | ||
- order_by: (Optional.) Specifies the order in which rows should be | ||
assigned to groups. Only affects the assignment of group numbers, not | ||
the ordering of the output rows. Defaults to the order of the rows in | ||
the table. | ||
Equal_Count (group_count:Integer=(Missing_Argument.throw "group_count")) (order_by:(Vector | Text)=[]) |
jdunkerley marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,50 @@ | ||||||
private | ||||||
|
||||||
from Standard.Base import all | ||||||
import Standard.Base.Errors.Common.Unsupported_Argument_Types | ||||||
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument | ||||||
|
||||||
import project.Column.Column | ||||||
import project.Grouping_Method.Grouping_Method | ||||||
import project.Internal.Java_Problems | ||||||
import project.Internal.Problem_Builder.Problem_Builder | ||||||
import project.Internal.Table_Helpers | ||||||
import project.Set_Mode.Set_Mode | ||||||
import project.Table.Table | ||||||
from project.Internal.Add_Row_Number import rename_columns_if_needed | ||||||
|
||||||
polyglot java import java.lang.ArithmeticException | ||||||
polyglot java import org.enso.table.operations.AddGroupNumber | ||||||
|
||||||
add_group_number (table:Table) (grouping_method:Grouping_Method) (name:Text) (from:Integer) (step:Integer) (on_problems:Problem_Behavior=..Report_Warning) -> Table = | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
problem_builder = Problem_Builder.new error_on_missing_columns=True | ||||||
|
||||||
handle_arithmetic_exception _ = | ||||||
Error.throw (Illegal_Argument.Error "The row number has exceeded the 64-bit integer range. BigInteger numbering is currently not supported. Please use a smaller start/step.") | ||||||
|
||||||
Panic.catch ArithmeticException handler=handle_arithmetic_exception <| Panic.catch Unsupported_Argument_Types handler=handle_arithmetic_exception <| | ||||||
Java_Problems.with_problem_aggregator on_problems java_problem_aggregator-> | ||||||
new_storage = case grouping_method of | ||||||
Grouping_Method.Unique group_by -> | ||||||
_illegal_if group_by.is_empty "..Unique requires a non-empty 'group_by'" <| | ||||||
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 < 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 | ||||||
renamed_table = rename_columns_if_needed table name on_problems Table.new | ||||||
problem_builder.attach_problems_before on_problems <| | ||||||
renamed_table.set new_column name set_mode=Set_Mode.Add | ||||||
|
||||||
_prepare_group_by table problem_builder group_by = | ||||||
table.columns_helper.select_columns_helper group_by Case_Sensitivity.Default True problem_builder . map c->c.java_column | ||||||
|
||||||
_prepare_ordering table problem_builder order_by = | ||||||
ordering = Table_Helpers.resolve_order_by table.columns order_by problem_builder | ||||||
ordering_columns = ordering.map c->c.column.java_column | ||||||
directions = ordering.map c->c.associated_selector.direction.to_sign | ||||||
[ordering_columns, directions] | ||||||
|
||||||
_illegal_if b msg ~cont = if b then Error.throw (Illegal_Argument.Error msg) else cont | ||||||
Comment on lines
+41
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should either mark the whole module There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Marked module as private. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to Missing_Argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want to default to
the Uniquesome grouping method?Perhaps
Equal_Count
is a good candidate for a default as it works regardless of what columns are there.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do have a default, I would say
Unique
because it's used much more.In general, do we prefer to have defaults for things, or is it also ok to have no default, which forces the user to choose, but also it shows up as a warning, which the user might not want to see every time they use this method. I'm fine either way -- no default, or
Unique [first column]
.