Skip to content

Add KDocs for public reverse APIs - #2050

Open
zaleslaw wants to merge 1 commit into
masterfrom
issue-1980
Open

Add KDocs for public reverse APIs#2050
zaleslaw wants to merge 1 commit into
masterfrom
issue-1980

Conversation

@zaleslaw

Copy link
Copy Markdown
Collaborator

Add KDocs for public reverse APIs

Closes #1980. All five public reverse overloads now have KDocs. No behavior changes.

KDoc

  • Added a shared KoDEx template CommonReverseDocs in api/reverse.kt. It holds the first
    line, the website link, @param [T] and @return. Each overload only sets its own parts
    (RECEIVER, UNIT, DETAILS, SEE_ALSO, TYPE_PARAM). Same pattern as CommonTakeAndDropDocs.
  • Added DocumentationUrls.Reverse for the reverse.html link.
  • The text describes the contract only — what you get back, not how it is done.

What each overload now states:

Overload Documented contract
DataFrame Only row order changes. Columns, types and schema stay the same.
DataColumn Name, type and kind stay the same. The result is typed as DataColumn, so for a column group you need asColumnGroup().
ColumnGroup Name and nested structure stay. Values in all nested columns stay aligned row-wise.
FrameColumn Only the order of the dataframes changes. Rows inside each dataframe keep their order.
ValueColumn Name and type stay the same.

Tests

3 tests -> 10. Every statement in the KDocs now has a test. The three old tests are unchanged.

The old tests did not call the overloads one may expect. columnOf(a, b) returns
DataColumn<DataRow<*>>, not ColumnGroup. columnOf(1, 2, 3) returns DataColumn<Int>, not
ValueColumn. So ColumnGroup.reverse() and ValueColumn.reverse() were never called by any
test. The new tests build these columns explicitly with DataColumn.createColumnGroup() and
DataColumn.createValueColumn().

New tests:

  • dataframe keeps schema and size — schema and row count.
  • column keeps name and type.
  • column overload keeps the column kind — a group stays Group, a frame stays Frame, a value
    stays Value. The DataColumn<*> types in this test are required: without them the calls go to
    the other overloads and the test would check something else.
  • column group type can be restored after the column overloadasColumnGroup() round-trip.
    ColumnGroup is not a subtype of DataColumn (it extends BaseColumn and DataFrame), so this
    cast is the only way to get the group type back.
  • column group reverses rows as a whole — real ColumnGroup receiver. It checks the name, the
    schema and the row pairs. It compares against explicit expected pairs and not against
    DataFrame.reverse(): comparing two implementations can pass while both are wrong.
  • frameColumn — the frames swap places, rows inside them keep their order.
  • valueColumn — the ValueColumn<Int> type of the result is the assertion here. The compiler
    checks it.

Note on assertions: column equality (impl/columns/Utils.kt) compares name, type and values. One
shouldBe on a whole column therefore already covers name and type. Separate name() / type()
asserts are used only where there is no whole-column comparison.

KDoc guidelines

Two notes added to KDOC_GUIDELINES.md, both found while writing the template:

  • A multi-line @set value keeps the indent of its continuation lines. An indent of 4+ spaces
    becomes a code block in Markdown, so the paragraph renders as monospace.
  • @param and @return must stay at the end of a KDoc. Text cannot be added after @include, so
    all parts that differ between overloads must be passed in as @set arguments.

…lumn, and ValueColumn

Implemented type-safe `reverse` operations for data structures, retaining metadata such as schema, column names, and column kinds. Updated KDocs and test cases accordingly.
@zaleslaw
zaleslaw requested review from Jolanrensen and a balanced review from Copilot August 26, 2026 15:12

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Comment thread KDOC_GUIDELINES.md
This makes the KDoc more refactor-safe, and it makes it easier to understand which arguments
need to be provided for a certain template.

A `@set` value may span several lines, but the indentation of the continuation lines is kept

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.

So, like:

/**
 *  {@get A}
 * {@set A   hello}
 */

?
I don't think this can happen, because the argument in @set is trimmed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ah, okey, agent faced several time with the problem and decided to include it, probably need to double-check

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.

can you give an example where this holds?

* Returns a new {@get [RECEIVER]} with the same {@get [UNIT]}s in reversed order,
* so the last {@get [UNIT]} becomes the first one.
*
* {@get [DETAILS]}

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.

When using a tag on a newline, no {} are needed


/**
* @include [CommonReverseDocs]
* {@set [CommonReverseDocs.RECEIVER] [DataFrame]}

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.

nowhere in this kdoc are {} necessary

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Will it work? Was it in the initial KDOC or KODEX guide?

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.

Yes, it works; block tags end when another block tag is encountered. I'm not sure how explicitly it's described, but it's how all other block tags work :)

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.

Brackets are only needed when you want to put a tag inline

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.

Normally you can see where a tag starts and ends using the intellij plugin of KoDEx, but it seems that has broken in the latest updates...

* @include [CommonReverseDocs]
* {@set [CommonReverseDocs.RECEIVER] [DataColumn]}
* {@set [CommonReverseDocs.UNIT] value}
* {@set [CommonReverseDocs.DETAILS] The column keeps its name, type and [kind][ColumnKind]:

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.

oxford comma missing

@Jolanrensen

Copy link
Copy Markdown
Collaborator

The produced KDocs look good and useful :)

@Jolanrensen
Jolanrensen self-requested a review August 27, 2026 15:01
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.

Add KDocs for public reverse APIs

3 participants