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 background_removal tool (skimage) #125

Merged
merged 9 commits into from
Jul 15, 2024
Merged

Conversation

rmassei
Copy link
Contributor

@rmassei rmassei commented Jul 12, 2024

Hi!
I think it would be nice to have in the collection a tool to automatically perform background removal from an intensity image.
This tool is based on skimage and more info can be found here.

At the present, three potential options:

  1. Rolling-Ball Algorithm
  2. Difference of Gaussians
  3. Top-Hat Filter

Original
img
Rolling-Ball
rolling
Difference of Gaussians
gaussian

Output is a 3 channel, 16 bit TIFF


FOR THE CONTRIBUTOR — Please fill out if applicable

Please make sure you have read the CONTRIBUTING.md document (last updated: 2024/04/23).

  • License permits unrestricted use (educational + commercial).

If this PR adds or updates a tool or tool collection:

@rmassei rmassei changed the title New Tool for Background Removal (skimage) Add background_removal tool (skimage) Jul 12, 2024
Copy link
Member

@kostrykin kostrykin left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this work!

There are a few comments below.

Also, please feel free to add yourself as a creator, as in:

<expand macro="creators/alliecreason"/>

tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
@bgruening
Copy link
Collaborator

Input staging problem: Test input file (img.png) cannot be found.

You can find the planemo output on this site for example: https://github.com/BMCV/galaxy-image-analysis/actions/runs/9907411219

Copy link
Contributor Author

@rmassei rmassei left a comment

Choose a reason for hiding this comment

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

Accepted all the changes and added the macro tests

Copy link
Member

@kostrykin kostrykin left a comment

Choose a reason for hiding this comment

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

Thanks @rmassei!

There are a few more comments below.

tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
macros/creators.xml Outdated Show resolved Hide resolved
tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
…loaded the new output with the 16bits input, fixed the creator.xml and tool xml
Copy link
Member

@kostrykin kostrykin left a comment

Choose a reason for hiding this comment

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

So I think there was a misunderstanding about my previous comment :)
#125 (comment)

I mean, the issue that the range of values is changed by the tool remains, even after you changed the test input data to an uint16 image. We need to dig a bit deeper here.

First, lets see how the wrapped functions change the data type:

So we only need to care for conversion to float. I suggest something like the above, but maybe it can be done more elegantly.

@rmassei
Copy link
Contributor Author

rmassei commented Jul 15, 2024

thanks @kostrykin, I did not get the problem in the first place.
Test were updated and it seems that the new function is working by correctly converting to the input data type.

Copy link
Member

@kostrykin kostrykin left a comment

Choose a reason for hiding this comment

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

Thanks @rmassei, I think we are almost done :) only a few minor comments below.

And there are a few flake8 linting issues which need to be addressed:
https://github.com/BMCV/galaxy-image-analysis/actions/runs/9937226470/job/27448034390?pr=125#step:6:1

tools/background_removal/background_removal.py:28:1: W293 blank line contains whitespace
tools/background_removal/background_removal.py:29:1: E302 expected 2 blank lines, found 1
tools/background_removal/background_removal.py:29:53: W291 trailing whitespace
tools/background_removal/background_removal.py:30:8: W291 trailing whitespace
tools/background_removal/background_removal.py:31:63: W291 trailing whitespace
tools/background_removal/background_removal.py:32:8: W291 trailing whitespace
tools/background_removal/background_removal.py:33:42: W291 trailing whitespace
tools/background_removal/background_removal.py:34:21: W291 trailing whitespace
tools/background_removal/background_removal.py:35:41: W291 trailing whitespace
tools/background_removal/background_removal.py:36:48: W291 trailing whitespace
tools/background_removal/background_removal.py:37:42: W291 trailing whitespace
tools/background_removal/background_removal.py:38:47: W291 trailing whitespace
tools/background_removal/background_removal.py:39:41: W291 trailing whitespace
tools/background_removal/background_removal.py:40:46: W291 trailing whitespace
tools/background_removal/background_removal.py:41:10: W291 trailing whitespace
tools/background_removal/background_removal.py:42:79: W291 trailing whitespace

tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
tools/background_removal/background_removal.xml Outdated Show resolved Hide resolved
Copy link
Member

@kostrykin kostrykin left a comment

Choose a reason for hiding this comment

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

Much thanks!

@kostrykin
Copy link
Member

https://github.com/BMCV/galaxy-image-analysis/actions/runs/9937821722?pr=125

pillow and tifffile are not installed, but required to compare files using the 'image_diff' method

I think this is due to #124
where we removed python -m pip install pillow tifffile in 6bc8575

@bernt-matthias Any idea how to fix this? Shall we add it back again?

@rmassei
Copy link
Contributor Author

rmassei commented Jul 15, 2024

yes, I got the same error locally before running python -m pip install pillow tifffile

@kostrykin kostrykin merged commit 004112a into BMCV:master Jul 15, 2024
10 checks passed
@kostrykin
Copy link
Member

Could not update background_removal
Failed to find repository for owner/name ufz/background_removal
Repository background_removal updated successfully.
Could not update background_removal
Failed to find repository for owner/name ufz/background_removal
Unexpected HTTP status code: 400: {"err_msg": "You already own a repository named background_removal, please choose a different name.", "err_code": 400008}
Repository [background_removal] does not exist in the targeted Tool Shed.
Failed to update metadata for repository 'background_removal' on the main Tool Shed.
Repository contents updated but failed to update metadata for repository 'background_removal' on the main Tool Shed.

https://github.com/BMCV/galaxy-image-analysis/actions/runs/9946388590/job/27477147950#step:6:222

Not sure what this error means. @bgruening do you know?

On the one hand it says that background_removal repository is already owned, on the other hand it complains that it doesn't exist… shouldn't it create it?

description: Background removal filters using scikit-image
long_description: Tool to perform a background removal using 1) Rolling-Ball Algorithm, 2) Difference of Gaussians and 3) Top-Hat Filter
name: background_removal
owner: ufz
Copy link
Collaborator

Choose a reason for hiding this comment

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

the owner is wrong, we can only push to imgteam

@bgruening
Copy link
Collaborator

Please change the owner in the shed.yml file

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