Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/with.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,60 @@ import org.jetbrains.kotlinx.dataframe.AnyColumnReference
import org.jetbrains.kotlinx.dataframe.DataFrame
import org.jetbrains.kotlinx.dataframe.DataRow
import org.jetbrains.kotlinx.dataframe.RowExpression
import org.jetbrains.kotlinx.dataframe.documentation.DocumentationUrls
import org.jetbrains.kotlinx.dataframe.documentation.ExcludeFromSources
import org.jetbrains.kotlinx.dataframe.impl.aggregation.internal
import org.jetbrains.kotlinx.dataframe.impl.aggregation.withExpr
import org.jetbrains.kotlinx.dataframe.impl.emptyPath
import kotlin.reflect.typeOf

// region docs

/**
* For each pivot cell: an empty group yields `null` (type `V?`), a group of exactly one row yields
* that single value (type `V`), and a group of several rows yields a `List<V>`.
*/
@ExcludeFromSources
private typealias PivotWithOneOrManyResultSnippet = Nothing

/**
* For each pivot cell: a group with no row selected by the reducer yields `null` (type `V?`),
* otherwise the value produced by the expression (type `V`).
*/
@ExcludeFromSources
private typealias PivotWithReducedResultSnippet = Nothing

/**
* If [expression\] returns a column reference, it is resolved to that column's value in the row
* the expression was evaluated on.
*/
@ExcludeFromSources
private typealias PivotWithColumnReferenceSnippet = Nothing

// endregion

// region Pivot

/**
* Aggregates this [Pivot] by evaluating the given [row expression][expression] on every row of each pivot group.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small addition might be to link to the Pivot Grammar just before the examples. Other than that I think it's quite complete

*
* @include [PivotWithOneOrManyResultSnippet]
*
* @include [PivotWithColumnReferenceSnippet]
*
* For more information: {@include [DocumentationUrls.Pivot]}
*

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some examples would be nice to see in each function. Maybe we already have them ready in :samples if they're used for the website. You can use @sample [fully qualified path to sample in :samples] to include a specific example. KoDEx will insert it when publishing

* Check out the [`Pivot` Grammar][PivotDocs.Grammar].
*
* ### Examples
*
* @sample [org.jetbrains.kotlinx.dataframe.samples.api.PivotSamples.withOnPivot_properties]
* @sample [org.jetbrains.kotlinx.dataframe.samples.api.PivotSamples.withOnPivot_strings]
*
* @param [V] The result type of [expression], captured at runtime for constructing the result columns.
* @param [expression] The expression evaluated for each row within each pivot group.
* @return A new [DataRow] with pivot keys as columns or column groups containing the computed results.
*/
public inline fun <T, reified V> Pivot<T>.with(noinline expression: RowExpression<T, V>): DataRow<T> =
delegate {
with(expression)
Expand All @@ -20,6 +67,25 @@ public inline fun <T, reified V> Pivot<T>.with(noinline expression: RowExpressio

// region ReducedPivot

/**
* Aggregates this [ReducedPivot] by evaluating the given [row expression][expression] on the single row
* selected by the reducer in each pivot group.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both reduced overloads describe the reducer as selecting the single row, with no mention of the no-row case — while the two non-reduced overloads now spell their result contract out via PivotWithOneOrManyResultSnippet. That asymmetry reads as "reduced always yields a V", but ReducedPivotGroupBy.with is reducer(this)?.let { … } (with.kt:138-140), so a group with no selected row yields null and the cell type widens to V?.

Failure scenario: df.groupBy { city }.pivot { isHappy }.maxBy { age }.with { name.lastName } on a sparse pivot — I ran it, and every key column comes back String? with null in the city/key combinations that have no rows (ReducedPivot.first { … } with no match is worse: Nothing?). A reader following this KDoc treats the cells as non-null String and gets an NPE or a failing cast/convert downstream. Sparse groupBy × pivot is the normal case, so this bites more often than the List<V> one you already documented.

Suggested fix: add a third shared fragment next to the existing two, e.g. "For each pivot cell: a group with no row selected by the reducer yields null (type V?), otherwise the value produced by the expression (type V).", and @include it in ReducedPivot.with and ReducedPivotGroupBy.with.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay this is a good note, afterwards I believe it's good to merge :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 6009beb: a shared PivotWithReducedResultSnippet fragment says a pivot cell whose group has no row selected by the reducer is null (type V?), otherwise the expression value (type V), and both ReducedPivot.with and ReducedPivotGroupBy.with include it.

The red "Build Preview of Generated Code" check isn't from this change. Its "Commit and push if changes" step fails with a 403 because the workflow token on a fork PR can't push to Kotlin/dataframe, and it failed the same way on the previous head.

*
* @include [PivotWithReducedResultSnippet]
*
* @include [PivotWithColumnReferenceSnippet]
*
* For more information: {@include [DocumentationUrls.Pivot]}
*
* ### Examples
*
* @sample [org.jetbrains.kotlinx.dataframe.samples.api.PivotSamples.pivotWith_properties]
* @sample [org.jetbrains.kotlinx.dataframe.samples.api.PivotSamples.pivotWith_strings]
*
* @param [V] The result type of [expression], captured at runtime for constructing the result columns.
* @param [expression] The expression evaluated on each selected row.
* @return A new [DataRow] with pivot keys as columns or column groups containing the computed results.
*/
public inline fun <T, reified V> ReducedPivot<T>.with(noinline expression: RowExpression<T, V>): DataRow<T> =
pivot.delegate {
reduce(reducer).with(expression)
Expand All @@ -29,6 +95,25 @@ public inline fun <T, reified V> ReducedPivot<T>.with(noinline expression: RowEx

// region PivotGroupBy

/**
* Aggregates this [PivotGroupBy] by evaluating the given [row expression][expression] on every row
* in each group defined by the grouping and pivot keys.
*
* @include [PivotWithOneOrManyResultSnippet]
*
* @include [PivotWithColumnReferenceSnippet]
*
* For more information: {@include [DocumentationUrls.Pivot]}
*
* ### Examples
*
* @sample [org.jetbrains.kotlinx.dataframe.samples.api.PivotSamples.withOnPivotGroupBy]
*
* @param [V] The result type of [expression], captured at runtime for constructing the result columns.
* @param [expression] The expression evaluated for each row within each pivot cell group.
* @return A new [DataFrame] with the grouping key columns, one row per grouping key combination,
* and pivot keys as columns or column groups containing the computed results.
*/
public inline fun <T, reified V> PivotGroupBy<T>.with(noinline expression: RowExpression<T, V>): DataFrame<T> {
val type = typeOf<V>()
return aggregate { internal().withExpr(type, emptyPath(), expression) }
Expand All @@ -38,6 +123,25 @@ public inline fun <T, reified V> PivotGroupBy<T>.with(noinline expression: RowEx

// region ReducedPivotGroupBy

/**
* Aggregates this [ReducedPivotGroupBy] by evaluating the given [row expression][expression] on the single row
* selected by the reducer in each group defined by the grouping and pivot keys.
*
* @include [PivotWithReducedResultSnippet]
*
* @include [PivotWithColumnReferenceSnippet]
*
* For more information: {@include [DocumentationUrls.Pivot]}
*
* ### Examples
*
* @sample [org.jetbrains.kotlinx.dataframe.samples.api.PivotSamples.withOnReducedPivotGroupBy]
*
* @param [V] The result type of [expression], captured at runtime for constructing the result columns.
* @param [expression] The expression evaluated on each selected row.
* @return A new [DataFrame] with the grouping key columns, one row per grouping key combination,
* and pivot keys as columns or column groups containing the computed results.
*/
public inline fun <T, reified V> ReducedPivotGroupBy<T>.with(noinline expression: RowExpression<T, V>): DataFrame<T> {
val type = typeOf<V>()
return pivot.aggregate {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,22 @@ class PivotSamples : DataFrameSampleHelper("pivot", "api") {
// SampleEnd
}

@Test
fun withOnPivotGroupBy() {
// SampleStart
df.groupBy { city }.pivot { isHappy }.with { name.lastName }
df.groupBy("city").pivot("isHappy").with { "name"["lastName"]<String>() }
// SampleEnd
}

@Test
fun withOnReducedPivotGroupBy() {
// SampleStart
df.groupBy { city }.pivot { isHappy }.maxBy { age }.with { name.lastName }
df.groupBy("city").pivot("isHappy").maxBy("age").with { "name"["lastName"]<String>() }
// SampleEnd
}

// endregion

// region aggregate
Expand Down
Loading