`indices()` on `BaseColumn` is ambiguous for a `ColumnGroup`
- Dominant language
- Kotlin
- Stars
- 1.1k
- Forks
- 83
- Avg merge
- 4d 12h
- Merged PRs (30d)
- 30
Description
### `indices()` on `BaseColumn` is ambiguous for a `ColumnGroup`
`indices()` exists on both `BaseColumn` and `DataFrame`:
| Declaration | Location |
|---|---|
| `public fun AnyBaseCol.indices(): IntRange = 0 until size()` | `core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/DataColumn.kt:325` |
| `public fun DataFrame<*>.indices(): IntRange = 0 until rowsCount()` | `core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/indices.kt:38` |
`ColumnGroup` derives from both (`core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/columns/ColumnGroup.kt`):
```kotlin
public interface ColumnGroup :
BaseColumn>,
DataFrame {
```
Neither candidate is more specific for a column group, so calling `indices()` on one does not compile.
#### Reproducer
Put this file at
`samples/src/test/kotlin/org/jetbrains/kotlinx/dataframe/samples/api/IndicesAmbiguityRepro.kt`:
```kotlin
package org.jetbrains.kotlinx.dataframe.samples.api
import org.jetbrains.kotlinx.dataframe.api.indices
import org.jetbrains.kotlinx.dataframe.indices
import org.junit.Test
class IndicesAmbiguityRepro : TestBase {
val df = peopleDf
@Test
fun repro() {
df.name.indices()
}
}
```
`df.name` is a `ColumnGroup` — `peopleDf` groups `firstName` and `lastName` into `name`.
Line 12 is `df.name.indices()`, column 17 is the `indices` token.
Run:
```
./gradlew :samples:compileTestKotlin
```
Output:
```
> Task :samples:compileTestKotlin FAILED
e: file:///.../samples/src/test/kotlin/org/jetbrains/kotlinx/dataframe/samples/api/IndicesAmbiguityRepro.kt:12:17 Overload resolution ambiguity between candidates:
fun DataFrame<*>.indices(): IntRange
fun BaseColumn<*>.indices(): IntRange
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':samples:compileTestKotlin' (registered by plugin 'org.jetbrains.kotlin.jvm').
> A failure occurred while executing org.jetbrains.kotlin.compilerRunner.btapi.BuildToolsApiCompilationWork
> Compilation error. See log for more details
```
#### Do not check this from inside `:core`
The same call **compiles** in `:core` when the calling file sits in package
`org.jetbrains.kotlinx.dataframe.api`. There `DataFrame.indices()` comes from the same package
and loses to the explicit `import org.jetbrains.kotlinx.dataframe.indices`, so only one candidate
is considered and the ambiguity never appears. A probe in `:core` returns `0..2` for a
three-row column group and looks perfectly healthy. `:samples` is the module that reflects a
user's import set.
#### The property form is not affected
`public val AnyCol.indices: IntRange` (`DataColumn.kt:317`) is declared on `AnyCol`, which is
`DataColumn<*>`. `ColumnGroup` does not derive from `DataColumn` — that is deliberate, per its
own KDoc: "It derives not from `DataColumn`, but from `BaseColumn` to avoid API clashes between
`DataFrame` and `DataColumn`." So the property simply does not apply to a column group, and
there is nothing to be ambiguous about. Only the function form is broken.
#### Why it happens
The placement rule is written in the types themselves.
`BaseColumn`:
> Column operations that doesn't clash by signature with `DataFrame` operations can be defined
> for `BaseColumn`
`DataColumn`:
> Column operations that have signature clash with `DataFrame` API (`filter`, `take`, `map` etc.)
> are defined for `DataColumn` and not for `BaseColumn`.
`indices()` does clash with `DataFrame.indices()`, so by that rule it belongs on `DataColumn`,
next to `filter`/`take`/`map`. Putting it on `BaseColumn` looks like an oversight.
#### Suggested fix
Narrow the receiver of `indices()` from `AnyBaseCol` to `AnyCol`. The function and the `indices`
property then agree on the receiver, and a column group resolves to `DataFrame.indices()` — which
returns the same range anyway, because `ColumnGroupImpl.size()` is `rowsCount()`
(`core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/columns/ColumnGroupImpl.kt`):
```kotlin
override fun size() = rowsCount()
```
Impact:
- `ValueColumn` and `FrameColumn` are `DataColumn`, so they are unaffected.
- The only in-repo use of `indices` on a `ColumnGroup` receiver is
`core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/reverse.kt:91`:
```kotlin
public fun ColumnGroup.reverse(): ColumnGroup = get(indices.reversed())
```
It uses the *property*. There are exactly two `indices` properties — `AnyCol.indices` and the
`internal val AnyFrame.indices` (`DataFrame.kt:182`) — and the first does not apply to a
column group, so this line already goes through the `DataFrame` side. No internal caller
depends on the function form for column groups.
- Narrowing the receiver of a public extension is source-breaking for any caller holding a
`BaseColumn`, so this needs an `apiDump` and probably a deprecation cycle rather than a
straight edit.
#### Current behaviour, for reference
Measured on a three-element column, and on a column group and a frame column built from a
three-row dataframe:
| Call | Result |
|---|---|
| `col.indices` | `0..2` |
| `col.indices()` | `0..2` |
| `base.indices()`, where `val base: AnyBaseCol = col` | `0..2` |
| `group.indices()` (from `:core`, where it compiles) | `0..2` |
| `group.size()` / `group.rowsCount()` | `3` / `3` |
| `frameCol.indices` / `frameCol.indices()` | `0..2` |
| `frameCol.size()` | `3` |
`IndicesTests` (`core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/indices.kt`, region
`on a column`) pins this down. The column-group case goes through a receiver explicitly typed as
`AnyBaseCol`, because that is the only way to call it today without hitting the ambiguity.
Tests are here: https://github.com/Kotlin/dataframe/pull/2068
Contributor guide
Assessment
This issue has not been assessed yet.