Skip to content

Added KDocs, tests and website examples for duplicate and duplicateRows - #2113

Open
zaleslaw wants to merge 2 commits into
masterfrom
issue-1962
Open

zaleslaw wants to merge 2 commits into
masterfrom
issue-1962

Conversation

@zaleslaw

Copy link
Copy Markdown
Collaborator

Closes #1962

duplicate and duplicateRows had no KDocs, no dedicated tests, and a website page without data examples. While covering them, we found that the four functions handled n <= 0 in four different ways, one of which silently lost data. This PR fixes that and documents the contract.

File Change
core/.../impl/api/duplicate.kt one n > 0 check at the entry of every overload
core/.../api/duplicate.kt KDocs for all four functions
core/.../documentation/DocumentationUrls.kt links to the three sections of the duplicate page
core/.../test/.../api/duplicate.kt new DuplicateTests
samples/.../api/DuplicateSamples.kt, samples/build.gradle.kts website samples; the page moved to :samples
docs/StardustDocs/topics/duplicate.md, _shadow_resources.md, resources/api/duplicate/ rewritten page with result tables
docs/StardustDocs/topics/appendDuplicate.md link text for duplicate

KDoc, page and tests use one dataset (name/age: Alice 15, Bob 20, Charlie 30).

Behaviour What it pins down
Row order copies follow their original row (A, A, B, B)
Condition only matching rows are repeated, other rows appear once
Nesting column groups and frame columns are repeated with their rows
Types duplicateRows keeps column types; DataRow.duplicate makes a nullable column non-nullable when the row value is not null
DataFrame.duplicate unnamed FrameColumn with n copies; .concat() gives all rows, then all rows again
n bounds n = 1 keeps one copy of everything; n <= 0 throws in all four functions

Note for the reviewer

Bugs fixed in the implementation (all four functions now throw IllegalArgumentException: Number of duplicates must be greater than 0, but was <n>):

  • duplicateRows(n) { condition } with n = 0 or a negative n silently removed every matching row: df.duplicateRows(-1) { age > 18 } returned only Alice and dropped Bob and Charlie, with no error. Pinned by duplicateRows with a condition rejects n that is not positive instead of removing matching rows.
  • DataFrame.duplicate(n) and DataRow.duplicate(n) with a negative n failed with IllegalArgumentException: Illegal Capacity: -1, an accidental message from ArrayList. With n = 0 they returned an empty FrameColumn or an empty DataFrame. Pinned by DataFrame duplicate rejects n that is not positive and DataRow duplicate rejects n that is not positive.
  • duplicateRows(n) checked n once per column, so a dataframe without columns accepted n = 0 without an error, while one with columns threw. Pinned by duplicateRows rejects n that is not positive.

All four tests failed before the fix.

The fix: one requirePositiveDuplicates(n) at the entry of each impl function, with the message the unconditional overload already used. For the overload with a condition, the check sits in the @PublishedApi duplicateRowsImpl(n, indices), not in the public inline body, so code compiled against the old inline function gets the check too. Signatures are unchanged, so the .api dump is unchanged as well.

Behaviour change for callers: duplicate(0), DataRow.duplicate(0) and duplicateRows(0) { … } now throw instead of returning an empty or reduced result. We chose to throw for n = 0 too, to match duplicateRows(n), which already threw.

Found and left as is: DataRow.duplicate takes each column's nullability from the row value: Int? becomes Int when the value is not null. This is now documented and pinned by a test.

Beyond the ticket:

  • The page moved from :core to :samples, because rendered result tables need DataFrameSampleHelper.
  • appendDuplicate.md is updated: its link text said "duplicate selected rows", but duplicate also repeats all rows and whole dataframes.
  • The section anchors in DocumentationUrls (#duplicaterows, #duplicate-on-a-datarow, #duplicate-on-a-dataframe) come from the new headings of this page.

This commit refactors duplicate functions to improve code structure and adds a new `requirePositiveDuplicates` check. Introduced HTML documentation for `duplicate` and `duplicateRows` APIs.

@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, thorough piece of work — approving, with one non-blocking note inline.

@zaleslaw
zaleslaw marked this pull request as ready for review October 1, 2026 13:58
@zaleslaw
zaleslaw requested a review from koperagen October 1, 2026 13:58

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve tests, KDocs, and site docs for duplicate / duplicateRows

1 participant