Skip to content

fix: remove python roundtrip in list ser/de - #244

Merged
ovesh merged 4 commits into
mainfrom
ref/change-list-serde
Jul 27, 2026
Merged

fix: remove python roundtrip in list ser/de#244
ovesh merged 4 commits into
mainfrom
ref/change-list-serde

Conversation

@declanyewlim

@declanyewlim declanyewlim commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Previous list serialization/deserialization paths treated used each cell as a separate call to the appropriate function. Since the serde functions took polars series as input, this meant every cell was converted from list -> pl.Series -> list. The time taken for this conversion is mostly inconsequential, but adds up over larger dataframes.

The new implementation flattens a column (of list of GK objects/structs) into a single list, converts that into a pl.Series, and runs that through the appropriate function before reconstructing the correct list lengths.

Empirically, this speeds up the serde paths by around 2-3 times:

# 1,000,000 row dataframe with lists (len between 2-8) of random GK Intervals
DataFrame creation time: 2.21293s
old serialization time: 124.13172s
newer serialization time: 62.46663s
---------------------------------
old deserialization time: 33.74061s
newer deserialization time: 13.30726s

Also makes polars options explicit when exploding lists of objects and correct None object serde.

@declanyewlim
declanyewlim requested a review from ovesh July 17, 2026 21:43
Comment thread genome_kit/df/serialization.py Outdated

return target_cols

def _explode_list(flattened: list[Any], orig_lengths: list[int]) -> list[list | None]:

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.

I think the action here is closer to unflatten than to explode, but I'm happy to be corrected if that's not the case. Also, maybe "explode" is always the right term to use in the context of dataframes.

@declanyewlim declanyewlim Jul 20, 2026

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.

Yep sorry "explode" is definitely not the right term here, that would be going in the other direction to a "long format".

"Implode" is technically correct either since that operates on a typically refers to aggregating over a column. "Unflatten" is probably the most precise term for this operation.

Comment thread genome_kit/df/serialization.py Outdated
dtype=pl.Object,
)
lengths = s.list.len().to_list()
flat = s.explode(empty_as_null=False)

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.

is flat the right var name here? (exploded != flat, right?)

Suggested change
flat = s.explode(empty_as_null=False)
exploded = s.explode(empty_as_null=False)

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.

Yep agree. The intention is that the result will be flat since we don't expect people to have nested lists of GK objects. I'll document that expectation somewhere as well.

(Nested lists of standard data types are supported in polars)

@declanyewlim

Copy link
Copy Markdown
Collaborator Author

Quick note on the serialization/deserialization paths for None values, None and [] are treated as the same in the code. When serializing then deserializing these values, the output in the dataframe will consistently be pl.Null (None in raw python). An empty list is technically not a missing value, but in this code is treated as so.

[None] is treated differently, and will be preserved on serialization/deserialization.

@declanyewlim
declanyewlim requested a review from ovesh July 20, 2026 15:27

@ovesh ovesh 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.

Please add tests

@declanyewlim declanyewlim changed the title refactor: change list deserialization function fix: remove python roundtrip in list serde Jul 27, 2026
@ovesh ovesh changed the title fix: remove python roundtrip in list serde fix: remove python roundtrip in list ser/de Jul 27, 2026
@ovesh
ovesh merged commit f5abf33 into main Jul 27, 2026
33 of 34 checks passed
@ovesh
ovesh deleted the ref/change-list-serde branch July 27, 2026 16:55
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.

2 participants