Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decorator for timelapse #10

Closed
wants to merge 72 commits into from
Closed

Decorator for timelapse #10

wants to merge 72 commits into from

Conversation

jo-mueller
Copy link
Member

@jo-mueller jo-mueller commented Apr 19, 2022

This PR adds a decorator that allows running functions on 4D points/surfaces/labels/etc.

Features

  • It inspects the called functions and checks the argument type annotations.
  • If it finds arguments of type napari.types.ImageData, LabelsData, PointsData or SurfaceData, it selects a suitable function to convert such 4D data to a list of 3D datasets which can then be processed frame by frame
  • Depending on the return type annotation (e.g., ImageData, SurfaceData, etc), the resulting list of 3D data is converted to a single 4D data object.

Currently supported:

supported_data = [ImageData, PointsData, SurfaceData, LabelsData]

Open issues:

The conversion functions for 4D -> 3D should check whether the data actually is 4D (or 3D or 2D) and handle this somehow. For this data, however, it works for now.

Tests

Functions are tested to a certain degree, but only very weakly.

@jo-mueller
Copy link
Member Author

@zoccoler if you have a few minutes to spare for this - the decorator running functions over multiple frames is the catchy part of this.

Does the current implementation make sense to you? What would be your preferred way to handle the "non" 4D-case?

Copy link
Contributor

@haesleinhuepf haesleinhuepf left a comment

Choose a reason for hiding this comment

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

Hi Johannes @jo-mueller ,

I was commenting here instead of #11 because it's more convenient to comment on code in PRs than in issues.

So this looks all very cool! I could imagine though that making something like a general Converter class + some specific PointConverter, etc may make sense.

Best,
Robert

setup.cfg Outdated Show resolved Hide resolved
src/napari_stress/_preprocess.py Outdated Show resolved Hide resolved
src/napari_stress/_surface.py Show resolved Hide resolved
src/napari_stress/_utils.py Outdated Show resolved Hide resolved
src/napari_stress/_utils.py Outdated Show resolved Hide resolved
src/napari_stress/_utils.py Outdated Show resolved Hide resolved
src/napari_stress/_tests/test_utils.py Outdated Show resolved Hide resolved
Only a rising edge should be discovered
I put them there for debugging
Function return depends on whether keywords were provided
@jo-mueller
Copy link
Member Author

To whom it may concern @haesleinhuepf @zoccoler @Cryaaa ,

I made quite a few changes to the code according to the suggestion above. A few notes to ease oversight:

  • New Converter class that convert lists of frames to a single layer object (Converter.list_of_data_to_data(...) and Converter.data_to_list_of_data(...)). It does not yet support all layer types as input (LayerDataTuples cannot be handled as input and VectorData does not work).
  • The Converter class is not yet able to differentiate between 3D and 4D data, which is crucial for further usage. I think this belongs in a separate PR, though.
  • There are quite some functions here that I needed for processing but of which I think that they ultimately belong in napari-process-points-and-surfaces in which I will move upstream in the next PRs
  • Most functions are not yet available as widgets in Napari but are only used from the notebooks. I think I will change this as soon as the time-slicing thing is resolved. For now, the functions serve their purpose primarily by being available from code.

@Cryaaa
Copy link

Cryaaa commented May 9, 2022

@jo-mueller, I don't know how feasible it is to use the time slicer functionality (or the concepts behind it) for points and surfaces. I really like the way 2D data is dealt with (t, z = 1, y, x), since this keeps things very uniform. Furthermore, the time slice will (soonish) have a version which directly generates a lazily loaded 4D dataset as a result. Maybe that would be something interesting? If you want to have a chat about 4D processing and approaches I'd be down :)

@jo-mueller
Copy link
Member Author

jo-mueller commented May 9, 2022

Hi @Cryaaa ,

Thanks for the Feedback. The original idea of this improvement was to create a simple version of the time slicer that would work with 4D points/surface data because the "real" timeslicer is currently not able to do this and I really need this sort of functionality for this project.

I hope it's possible to merge this with the real timeslicer somewhere down the road.

I'll try to split this into several PRs so that it becomes clearer what is happening where.

@jo-mueller
Copy link
Member Author

I'm closing this PR to be continued in a more concise thread PR #18

@jo-mueller jo-mueller closed this May 9, 2022
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.

None yet

3 participants