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

Add the option to have an image path inside index file #349

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

fefossa
Copy link
Contributor

@fefossa fefossa commented Jun 15, 2023

This PR adds the possibility for the user to give a full path to the image file inside the index.csv file. It's just checking if the FileName is a path; if it's a path, it won't add the root to the FileName.

The user may want to give a full path to the image file instead of moving the images to inputs/images. DeepProfiler expects to have the images inside "inputs/images", and it always adds the root path to the filenames.

Mostly for RunDeepProfiler (discussion and PR) to work on CellProfiler we need to have this function so we don't need to move the images to inputs/images and grab it from the user's inputs.

I've tested this also by giving the inputs as DeepProfiler usually expects (index.csv only having "Plate/FileName") and it worked fine.

The user may want to give a full path to the image file instead of moving the images to inputs/images.
@bethac07
Copy link
Member

bethac07 commented Aug 4, 2023

@jccaicedo @Arkkienkeli Any objection to merging this? We'd love to get the CellProfiler DeepProfiler plugin merged? Thanks!

@Arkkienkeli
Copy link
Member

@fefossa Hi, did you test it for both full paths and inputs/image paths?

@fefossa
Copy link
Contributor Author

fefossa commented Aug 7, 2023

@Arkkienkeli Yes, I've tested this also by giving the inputs as DeepProfiler usually expects (index.csv only having "Plate/FileName", with the images inside inputs/image) and it worked fine.

@Arkkienkeli
Copy link
Member

@fefossa
Do you mind to document this change in the handbook? Specifically, here. I guess, we can merge after that.

@fefossa
Copy link
Contributor Author

fefossa commented Dec 19, 2023

It took me some time, sorry - but I've modified the documentation as requested by @Arkkienkeli. Thanks!

@Arkkienkeli Arkkienkeli merged commit 6914fc9 into cytomining:master Jan 2, 2024
@Arkkienkeli
Copy link
Member

Thank you, I have merged both documentation and this PRs.

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.

3 participants