Table: grouping #1392
Merged
Table: grouping #1392
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -262,10 +262,118 @@ type Column | |||
| fields = Map.singleton "name" (Json.String name) . insert "data" storage_json | |||
| Json.Object fields | |||
|
|
|||
| ## Efficiently joins two tables based on either the index or a key column. | |||
There was a problem hiding this comment.
Suggested change
| ## Efficiently joins two tables based on either the index or a key column. | |
| ## Efficiently joins two tables based on either the index or the specified key column. |
Comment on lines
268
to
269
| `other` with matching indexes. If the index in `other` is not unique, | ||
| the corresponding rows of `this` will be duplicated in the result. |
There was a problem hiding this comment.
This sentence seems a bit unclear to me. Maybe it would be clearer with something like: "If the column corresponding to this' index in other is not unique..."?
Comment on lines
302
to
303
| - function: the function used for value aggregation. Values belonging | ||
| to each grouped are passed to this function in a vector. |
There was a problem hiding this comment.
This sentence doesn't make sense. Do you mean "belonging to each group"?
Comment on lines
+359
to
+364
| mean : Text -> Column | ||
| mean name_suffix='_mean' = | ||
| vec_mean v = if v.length == 0 then Nothing else | ||
| (Vector.Vector v).reduce (+) / v.length | ||
| r = this.java_column.aggregate ['mean', name_suffix, vec_mean, True] | ||
| Column r |
There was a problem hiding this comment.
Probably worth adding mode and median as fast operations alongside mean.
|
|
||
| Arguments: | ||
| - show_rows: the number of initial rows that should be displayed. | ||
| print show_rows=10 = this.values.print show_rows |
There was a problem hiding this comment.
Missing type signature.
| Object _this, Object src, long source_index, Array that, long dest_index, long count) { | ||
| Builtins builtins = lookupContextReference(Language.class).get().getBuiltins(); | ||
| throw new PanicException( | ||
| builtins.error().typeError().newInstance(builtins.mutable().array().newInstance(), src), |
iamrecursion
approved these changes
Jan 11, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Introduces grouping to tables
Important Notes
Checklist
Please include the following checklist in your PR: