Repository navigation
Add KDocs for split - #2189
Add KDocs for split#2189Jolanrensen wants to merge 8 commits into
Conversation
Co-authored-by: Junie <junie@jetbrains.com>
There was a problem hiding this comment.
Thanks a lot for this, just one blocking fix needed 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=1b6cd04c-d063-4f5a-80f2-828701dcdb97
6c94752 to
edb423e
Compare
There was a problem hiding this comment.
Nearly there, just one blocking doc fix left 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=b03f2dac-cb39-4c8d-b00b-d10cc70d78cd
| * How many new columns are created depends on the maximum number of parts among the values of the original column, | ||
| * or the number of the given [names\]. | ||
| * Values that are split into fewer parts than the number of new columns get $[MISSING] in the remaining columns. | ||
| * `null` values cannot be split, so they become `null` in all new columns. |
There was a problem hiding this comment.
Blocking: the last commit changed this to say null values always become null, which is wrong. by maps null to emptyList() (line 608), and splitImpl pads every missing slot with clause.default. So null values get $[MISSING], the same as short values.
Failure: split { a }.by(",").default("-").into(...) fills null rows with "-", but this snippet (included in into/inward) says null. It also contradicts both default KDocs in this file ("as well as null values, get [value]").
Fix: go back to the previous wording, e.g. "null values are split into no parts, so they get $[MISSING] in all new columns."
There was a problem hiding this comment.
Thanks for the update, the HTML is in now; just one blocking fix left 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=cb30e272-c5fe-4033-b2ad-d0e3785535c3
| * How many new columns are created depends on the maximum number of parts among the values of the original column, | ||
| * or the number of the given [names\]. | ||
| * Values that are split into fewer parts than the number of new columns get $[MISSING] in the remaining columns. | ||
| * `null` values cannot be split, so they become `null` in all new columns. |
There was a problem hiding this comment.
Blocking: the change in the last commit made this wrong. null values do not always become null. by(type, splitter) maps null to emptyList(), and then splitImpl fills every new column with clause.default.
Failure: split { a }.by(',').default("-").into("x", "y") gives "-" in both columns for a null cell. This snippet is included into every into/inward overload and says the result is null. It also contradicts the KDoc of default ("as well as null values, get [value]").
Fix: go back to the previous wording: "null values are split into no parts, so they get $[MISSING] in all new columns."
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=cb30e272-c5fe-4033-b2ad-d0e3785535c3
Add KDocs for
splitFixes #1471
Added all missing kdocs for split.kt, moved website examples to :samples so they could be reused here.
Good one! Jolanrensen/KoDEx#118