Skip to content

Integrating Caked to Affinity#293

Merged
crangelsmith merged 29 commits intodevelopfrom
caked-integration
Apr 8, 2024
Merged

Integrating Caked to Affinity#293
crangelsmith merged 29 commits intodevelopfrom
caked-integration

Conversation

@crangelsmith
Copy link
Collaborator

@crangelsmith crangelsmith commented Feb 26, 2024

Caked is being developed to allow for a more flexible data loader. It should do everything that is already implemented in affinity and more (e.g integration with CZI portal, parakeet, etc).

This PR, integrates Caked with affinity, making the data.py a lot lighter in code.

Commit history is a mess due to me trying to keep up to date with develop, but the files changed are the right ones to review.

@crangelsmith crangelsmith changed the title [WIP] adding caked to the installations [WIP] Integrating Caked to Affinity Feb 26, 2024
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@crangelsmith crangelsmith changed the base branch from develop to napari-widget February 27, 2024 15:55
@crangelsmith crangelsmith changed the base branch from napari-widget to develop February 27, 2024 15:55
@crangelsmith crangelsmith reopened this Feb 27, 2024
@crangelsmith crangelsmith mentioned this pull request Mar 1, 2024
4 tasks
@crangelsmith crangelsmith changed the title [WIP] Integrating Caked to Affinity Integrating Caked to Affinity Mar 6, 2024
@crangelsmith crangelsmith changed the base branch from develop to main March 6, 2024 11:29
@crangelsmith crangelsmith changed the base branch from main to develop March 6, 2024 11:29
Copy link
Collaborator

@marjanfamili marjanfamili left a comment

Choose a reason for hiding this comment

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

Would it be possible to have a docstring in the overload load_dat() functions in data.py to explain why they are necessary? I am assuming is it for calling in evaluation mode ?

Copy link
Collaborator

@marjanfamili marjanfamili left a comment

Choose a reason for hiding this comment

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

The code runs fine, however I find all the new additions slightly confusing, specially because one has to switch back and force between caked and affinity to see what all the functions are. it would be good to have some short comments to explain what is happening when they are being used.

I have tested this on Mac for Mnist data with few transformations. The installation of caked works well in existing python environment without any conflict.

x = self.voxel_transformation(data)
# read data from path
data = np.array(self.read(self.paths[index]))
x = self.transformation(data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is self.transformation() ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is the equivalent to voxel_transformation, but as we should be able to deal with very different datasets (1D, 2D and 3D) it was more clear to leave it a transformation only.

@crangelsmith
Copy link
Collaborator Author

Would it be possible to have a docstring in the overload load_dat() functions in data.py to explain why they are necessary? I am assuming is it for calling in evaluation mode ?

Done in #88bbd47

@crangelsmith
Copy link
Collaborator Author

The code runs fine, however I find all the new additions slightly confusing, specially because one has to switch back and force between caked and affinity to see what all the functions are. it would be good to have some short comments to explain what is happening when they are being used.

I have tested this on Mac for Mnist data with few transformations. The installation of caked works well in existing python environment without any conflict.

Added more comments for context in 88bbd47

@crangelsmith crangelsmith merged commit 8631791 into develop Apr 8, 2024
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

Comments