Skip to content

Refactor reading and writing, keep scale and offset. - #42

Draft
georgeoshardo wants to merge 6 commits into
mainfrom
refactor
Draft

Refactor reading and writing, keep scale and offset. #42
georgeoshardo wants to merge 6 commits into
mainfrom
refactor

Conversation

@georgeoshardo

Copy link
Copy Markdown
Collaborator

References and relevant issues

I started out with the goal of addressing #38 but also ended up fixing #19

Description

Pull a lot of the reader logic out into _features.py to create this dataframe that we've been using for the tracks layer. But now no longer rely solely on this cursed dataframe to reconstruct the geff if we don't need to. This also solved my problem of bools and other types getting coerced for properties which weren't on all nodes/edges.

Also, if any edges/nodes are added/removed after the geff has been loaded, these should be saved out.

We also write out the scale and offset to the geff now.

_features.py builds pandas df from zarr. Refactors _reader.py to use this, simplifies tracks data preparation for napari.

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

Sorry for the delay on this one @georgeoshardo, I was traveling. I have a quick question before I look at this in more depth.

But now no longer rely solely on this cursed dataframe to reconstruct the geff if we don't need to.

Can you please expand on this a bit?

import zarr


def _pd_nullable_from_str(dtype_str: str) -> Any:

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 have some serious concerns about this one 😬 because it seems brittle and difficult to maintain.

Where does GEFF get its dtypes from? Is it dependent on what library was used to write the GEFF initially? Presumably, the list we're checking against here is not exhaustive, right?

I think we should defer to pandas' own type inference API because it's more likely to be correct, and it'll be updated with future versions of pandas. WDYT?

c for c in prop_names if c in data_cols and c != "node_id"
]
df = pd.DataFrame(data_cols)
print("Here")

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.

Debugging leftover?

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