Skip to content

Migrate usages of AccessOverload APIs in inline functions to public alternatives #1162

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

Merged
merged 1 commit into from
May 5, 2025
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
1 change: 1 addition & 0 deletions core/api/core.api
Original file line number Diff line number Diff line change
Expand Up @@ -2548,6 +2548,7 @@ public final class org/jetbrains/kotlinx/dataframe/api/IntoKt {
public static final fun into (Lorg/jetbrains/kotlinx/dataframe/api/GroupBy;Ljava/lang/String;)Lorg/jetbrains/kotlinx/dataframe/DataFrame;
public static final fun into (Lorg/jetbrains/kotlinx/dataframe/api/GroupBy;Lkotlin/reflect/KProperty;)Lorg/jetbrains/kotlinx/dataframe/DataFrame;
public static final fun into (Lorg/jetbrains/kotlinx/dataframe/api/GroupBy;Lorg/jetbrains/kotlinx/dataframe/columns/ColumnAccessor;)Lorg/jetbrains/kotlinx/dataframe/DataFrame;
public static final fun into (Lorg/jetbrains/kotlinx/dataframe/api/GroupBy;Lorg/jetbrains/kotlinx/dataframe/columns/ColumnPath;Lkotlin/jvm/functions/Function2;Lkotlin/reflect/KType;)Lorg/jetbrains/kotlinx/dataframe/DataFrame;
public static final fun into (Lorg/jetbrains/kotlinx/dataframe/api/ReducedGroupBy;Ljava/lang/String;)Lorg/jetbrains/kotlinx/dataframe/DataFrame;
public static final fun into (Lorg/jetbrains/kotlinx/dataframe/api/ReducedGroupBy;Lkotlin/reflect/KProperty;)Lorg/jetbrains/kotlinx/dataframe/DataFrame;
public static final fun into (Lorg/jetbrains/kotlinx/dataframe/api/ReducedGroupBy;Lorg/jetbrains/kotlinx/dataframe/columns/ColumnAccessor;)Lorg/jetbrains/kotlinx/dataframe/DataFrame;
Expand Down
14 changes: 13 additions & 1 deletion core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/into.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import org.jetbrains.kotlinx.dataframe.annotations.AccessApiOverload
import org.jetbrains.kotlinx.dataframe.annotations.Interpretable
import org.jetbrains.kotlinx.dataframe.annotations.Refine
import org.jetbrains.kotlinx.dataframe.columns.ColumnAccessor
import org.jetbrains.kotlinx.dataframe.columns.ColumnPath
import org.jetbrains.kotlinx.dataframe.impl.aggregation.internal
import org.jetbrains.kotlinx.dataframe.impl.aggregation.withExpr
import org.jetbrains.kotlinx.dataframe.impl.columnName
import kotlin.reflect.KProperty
import kotlin.reflect.KType
import kotlin.reflect.typeOf

// region GroupBy
Expand All @@ -29,7 +31,7 @@ public fun <T> GroupBy<T, *>.into(column: KProperty<AnyFrame>): DataFrame<T> = t
public inline fun <T, G, reified V> GroupBy<T, G>.into(
columnName: String? = null,
noinline expression: RowExpression<G, V>,
): DataFrame<G> = into(pathOf(columnName ?: groups.name()).cast(), expression)
): DataFrame<G> = into(pathOf(columnName ?: groups.name()), expression, typeOf<V>())

// @Hide
@AccessApiOverload
Expand All @@ -44,6 +46,16 @@ public inline fun <T, G, reified V> GroupBy<T, G>.into(
}
}

@PublishedApi
internal fun <T, G, V> GroupBy<T, G>.into(
path: ColumnPath,
expression: RowExpression<G, V>,
type: KType,
): DataFrame<G> =
aggregate {
internal().withExpr(type, path, expression)
}

@AccessApiOverload
public inline fun <T, G, reified V> GroupBy<T, G>.into(
column: KProperty<V>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public inline fun <reified T> Iterable<T>.toDataFrame(): DataFrame<T> =
// or has no properties
if (!T::class.canBeUnfolded) {
// create a single `value` column
ValueProperty<T>::value from { it }
ValueProperty<T>::value.name from { it }
} else {
// otherwise creates columns based on properties
properties()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public inline fun <T, reified V> ReducedPivotGroupBy<T>.with(noinline expression
val value = reducer(this)?.let {
val value = expression(it, it)
if (value is AnyColumnReference) {
it[value]
value.getValue(it)
} else {
value
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ internal fun <T, V> AggregateInternalDsl<T>.withExpr(type: KType, path: ColumnPa
val values = df.rows().map {
val value = expression(it, it)
if (value is AnyColumnReference) {
it[value]
value.getValue(it)
} else {
value
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ internal inline fun <T, reified V : R, R> Aggregator<V & Any, R>.aggregateByOrNu
): DataRow<T>? =
data.getOrNull(
indexOfAggregationResult(
values = data.asSequence().map { it[column] },
values = data.asSequence().map { column.getValue(it) },
valueType = typeOf<V>(),
),
)
Original file line number Diff line number Diff line change
Expand Up @@ -1176,8 +1176,9 @@ class DataFrameTests : BaseTest() {
.split { others }.intoRows()
.add(sum) { name.length + other().length }

val matrix = src.pivot { other }.groupBy { name }.with { sum }
val matrix = src.pivot { other }.groupBy { name }.with { sum() }
matrix.getColumnGroup(other.name()).ncol shouldBe names.size
matrix.getColumnGroup(other.name())["Alice"].type() shouldBe typeOf<List<Int>>()
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,13 @@ class PivotTests {
.groupBy { name }
.default("-")
.values { value } shouldBe res
typed
.pivot { key }
.groupBy { name }
.default("-")
.with { value() } shouldBe res

// special case that looks very odd, but kept it for compatibility
typed
.pivot { key }
.groupBy { name }
Expand All @@ -180,7 +187,7 @@ class PivotTests {
.pivot(key)
.groupBy(name)
.default("-")
.with { value } shouldBe res
.with { value() } shouldBe res
typed
.groupBy { name }
.pivot { key }
Expand Down