Skip to content

Fixed incorrect columns in valueCounts of 0 rows - #2168

Merged
Jolanrensen merged 4 commits into
Kotlin:masterfrom
Kanish2002:fix/empty-value-counts-schema
Oct 21, 2026
Merged

Jolanrensen merged 4 commits into
Kotlin:masterfrom
Kanish2002:fix/empty-value-counts-schema

Conversation

@Kanish2002

@Kanish2002 Kanish2002 commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

When no observations remain, valueCounts rebuilds its observed-value column from an empty list. For a ColumnGroup, this loses the child columns. DataFrame counting uses a temporary group, so df.take(0).valueCounts("city") can return [rows, count] rather than [city, count].

Use an empty slice for structured columns when there are no distinct values. Value columns keep the existing type-aware construction, preserving their runtime nullability according to dropNA. Nonempty counting and public signatures are unchanged.

Regression coverage includes empty all-column/selected-column frames, all rows excluded by dropNA using a genuinely nullable String fixture, an empty ColumnGroup with non-nullable children, and empty nullable value columns with dropNA both enabled and disabled. The website description documents the empty result.

Fixes #2167.

Validation:

  • Isolated Kotlin/JUnit: all 32 ValueCountsTests pass. The new empty nullable value-column regression fails on the previous PR head and passes with this update.
  • Compiled the actual API source, unchanged current annotations/Plugin.kt, and unchanged current impl/columns/ValueColumnImpl.kt with Kotlin 2.4.21, targeting JVM 8, against published dataframe-core 1.0.0-rc01. Including the current implementation exercises its current slicing/nullability behavior rather than the older published implementation. Tests target JVM 11; friend paths expose library internals.
  • Standalone ktlint and git diff --check pass.
  • The repository debug test command was attempted, but the Gradle 9.8.1 distribution download is blocked by network access. Full-module/debug validation and KDoc processing remain unverified locally.

Found while preparing #2165, but independent of that API proposal. AI-assisted.

@Jolanrensen
Jolanrensen self-requested a review October 20, 2026 11:02
@Jolanrensen Jolanrensen added the bug Something isn't working label Oct 20, 2026
@Jolanrensen

Copy link
Copy Markdown
Collaborator

@Kanish2002 thanks for the bugfix! I'll take it from here :)

@Jolanrensen Jolanrensen self-assigned this Oct 20, 2026
@Jolanrensen Jolanrensen added this to the 1.0 milestone Oct 20, 2026
@Jolanrensen Jolanrensen changed the title Preserve observed columns in empty valueCounts results Fixed incorrect columns in valueCounts of 0 rows Oct 20, 2026
@Jolanrensen
Jolanrensen removed their request for review October 20, 2026 12:44
@Jolanrensen

Copy link
Copy Markdown
Collaborator

Made some tiny fixes, but we need to merge the fix for #2034 first so the tests pass as expected :)

@Jolanrensen
Jolanrensen marked this pull request as ready for review October 20, 2026 13:27

@jetbrains-air jetbrains-air Bot left a comment

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.

Nice, targeted fix — just two test expectations to sort out before this can go green. 🙂

Produced by Air Automations. Name: AI Review / Run: https://air.jetbrains.cloud/org/05cf1a7f-6ab5-713b-abd3-29d0c8a05e2d/automations/8daf2412-0b6c-4433-90c0-b40ab5aebcdb?run=07e7bda9-a504-4728-a72f-b4f3d3205e0b

Comment thread core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/valueCounts.kt Outdated
Comment thread core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/valueCounts.kt Outdated

@jetbrains-air jetbrains-air Bot left a comment

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.

Thanks for digging into this one — the structure-preserving slice is the right call; just one blocking note inline before merge.

Produced by Air Automations. Name: AI Review / Run: https://air.jetbrains.cloud/org/05cf1a7f-6ab5-713b-abd3-29d0c8a05e2d/automations/8daf2412-0b6c-4433-90c0-b40ab5aebcdb?run=6738ebad-f9e5-4432-be63-d5347fbb1134

Comment thread core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/valueCounts.kt Outdated
@Jolanrensen
Jolanrensen force-pushed the fix/empty-value-counts-schema branch from 4397774 to f1eaac1 Compare October 20, 2026 14:19
@Jolanrensen
Jolanrensen force-pushed the fix/empty-value-counts-schema branch from f1eaac1 to efef26e Compare October 20, 2026 14:20
@Jolanrensen

Copy link
Copy Markdown
Collaborator

@Kanish2002 thanks, I'll run the CI

@Jolanrensen
Jolanrensen merged commit 5016a06 into Kotlin:master Oct 21, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

valueCounts loses observed columns when no observations are counted

3 participants