Repository navigation
String api docs update - #2166
AndreiKingsley wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The documentation matches existing APIs and samples, with no unresolved issues.
No open issues 🎉
What changed in this PR
Clarifies Kotlin DataFrame’s Invoked String API documentation without changing runtime behavior.
Changes:
- Adds a comparison table mapping string column accessors to invoked-string syntax.
- Removes the “outdated” label while retaining the change warning and discouraging production use.
| File | Description |
|---|---|
| docs/StardustDocs/topics/concepts/StringApi.md | Clarifies syntax equivalents and usage guidance. |
Review effort: Balanced
![jetbrains-air[bot]](https://avatars.githubusercontent.com/in/3314082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nice clarification of the Invoked String API — looks good to me, approving!
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=77f28bcb-7dbb-444f-80d8-63b2ca0ee6fa
| | `col<String>("name")` / `getValue<String>("name")` | `"name"<String>()` | | ||
| | `colGroup("info").col("age")` | `"info"["age"]` | | ||
| | `colGroup("info").col<Int>("age")` / `getColumnGroup("info").getValue<Int>("age")` | `"info"["age"]<Int>()` | | ||
| | `colGroup("info").cols("age", "height")` | `"info"["age", "height"]` | |
There was a problem hiding this comment.
Problem: This row holds only in the Columns Selection DSL, but the section documents row expressions too (the sample right above has a "Row Expressions" half). "a"["b", "c"] resolves to a different overload in each context: ColumnsSelectionDsl.String.get(firstCol, vararg otherCols): ColumnSet<*> (core/.../api/cols.kt:598) selects two sibling columns, while DataRow.String.get(vararg path): ColumnPath (core/.../DataRow.kt:92) builds the nested path info/age/height. The other CS-DSL-only rows are harmless because they have no row-expression overload at all; this one silently has a conflicting one.
Failure scenario: A reader copies the cell into a row expression — df.add("x") { "info"["age", "height"]<Int>() }. It compiles (DataRow.ColumnPath.invoke<R>() is in scope) and throws at runtime on a missing info/age/height column. No compile-time signal, and the error names a path the user never wrote.
Suggested fix: Scope the table to the Columns Selection DSL (rows 2 and 4 already carry their row-expression counterparts explicitly), and add one line under it noting that in row expressions "a"["b", "c"] builds the nested path a/b/c instead. This is the CS-DSL-vs-row-expression inconsistency already tracked in #1684, so documenting it here is cheap.
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=3d14bcc2-e95a-4c73-ae7d-7cf264ce8818
There was a problem hiding this comment.
Nice clarification of the Invoked String API — the new correspondence table checks out, so this looks good to 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=93ef18e4-322f-4486-984e-e5c15c1c4adc
No description provided.