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

Refurbishment #118

Merged
merged 3 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
**For the contributor:**
* [ ] I have read the [CONTRIBUTING.md](https://github.com/BMCV/galaxy-image-analysis/blob/master/CONTRIBUTING.md) document.
* [ ] License permits unrestricted use (educational + commercial).
* [ ] This PR adds or updates a tool or tool collection.
* [ ] This PR adds a new tool or tool collection.
* [ ] This PR updates an existing tool or tool collection.
* [ ] Tools added/updated by this PR comply with the [Naming and Annotation Conventions for Tools in the Image Community in Galaxy](https://github.com/elixir-europe/biohackathon-projects-2023/blob/main/16/paper/paper.md#conventions) (or explain why they do not).
* [ ] This PR does something else (explain below).

---

<!-- Please describe your PR here -->
**FOR THE CONTRIBUTOR — Please fill out if applicable**

Please make sure you have read the [CONTRIBUTING.md](https://github.com/BMCV/galaxy-image-analysis/blob/master/CONTRIBUTING.md) document (last updated: 2024/03/18).

* [ ] License permits unrestricted use (educational + commercial).

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

* [ ] This PR adds a new tool or tool collection.
* [ ] This PR updates an existing tool or tool collection.
* [ ] Tools added/updated by this PR comply with the [Naming and Annotation Conventions for Tools in the Image Community in Galaxy](https://github.com/elixir-europe/biohackathon-projects-2023/blob/main/16/paper/paper.md#conventions) (or explain why they do not).
10 changes: 5 additions & 5 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ jobs:
tool-list: ${{ needs.setup.outputs.tool-list }}
additional-planemo-options: --biocontainers -s tests,output,inputs,help,general,command,citations,tool_xsd,xml_order,tool_urls,shed_metadata
- run: |
python -m pip install git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/util
python -m pip install git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/tool_util
python -m pip install git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/util
python -m pip install git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/tool_util
#
# ---------------------------------------------------------
#
Expand Down Expand Up @@ -185,9 +185,9 @@ jobs:
chunk: ${{ matrix.chunk }}
chunk-count: ${{ needs.setup.outputs.chunk-count }}
- run: |
python -m pip install git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/util
python -m pip install git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/tool_util
python -m pip install pillow
python -m pip install git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/util
python -m pip install git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/tool_util
python -m pip install pillow tifffile
#
# ---------------------------------------------------------
#
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ jobs:
# - https://github.com/galaxyproject/galaxy/pull/17581
#
- run: |
python -m pip install --no-dependencies git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/util
python -m pip install --no-dependencies git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/tool_util
python -m pip install --no-dependencies git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/util
python -m pip install --no-dependencies git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/tool_util
#
# ---------------------------------------------------------
#
Expand Down Expand Up @@ -311,9 +311,9 @@ jobs:
# - https://github.com/galaxyproject/galaxy/pull/17581
#
- run: |
python -m pip install --no-dependencies git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/util
python -m pip install --no-dependencies git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/tool_util
python -m pip install pillow
python -m pip install --no-dependencies git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/util
python -m pip install --no-dependencies git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/tool_util
python -m pip install pillow tifffile
#
# ---------------------------------------------------------
#
Expand Down
88 changes: 75 additions & 13 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,36 +1,98 @@
# Contributing

This document describes how to contribute to this repository. Pull
requests containing bug fixes, updates, and extensions to the existing
tools and tool suites in this repository will be considered for
inclusion.
This document is the attempt to collect some rough rules for tools to follow in this repository, to facilitate their consistency and interoperability. This document is an extension to the [Naming and Annotation Conventions for Tools in the Image Community in Galaxy](https://github.com/elixir-europe/biohackathon-projects-2023/blob/main/16/paper/paper.md#conventions) and compatibility should be maintained. This document is work in progress.

## How to Contribute
**How to Contribute:**

* Make sure you have a [GitHub account](https://github.com/signup/free)
* Make sure you have git [installed](https://help.github.com/articles/set-up-git)
* Fork the repository on [GitHub](https://github.com/BMCV/galaxy-image-analysis/fork)
* Make the desired modifications - consider using a [feature branch](https://github.com/Kunena/Kunena-Forum/wiki/Create-a-new-branch-with-git-and-manage-branches).
* Try to stick to the [Conventions for Tools in the Image Community](https://github.com/elixir-europe/biohackathon-projects-2023/blob/main/16/conventions.md) and the [IUC standards](http://galaxy-iuc-standards.readthedocs.org/en/latest/) whenever you can
* Try to stick to the [Conventions for Tools in the Image Community](https://github.com/elixir-europe/biohackathon-projects-2023/blob/main/16/paper/paper.md#conventions) and the [IUC standards](http://galaxy-iuc-standards.readthedocs.org/en/latest/) whenever you can
* Make sure you have added the necessary tests for your changes and they pass.
* Open a [pull request](https://help.github.com/articles/using-pull-requests) with these changes.

## Using the new image-based verifications for tool testing
## Terminology

The new image-based verifications for Galaxy tool tests https://github.com/galaxyproject/galaxy/pull/17556 and https://github.com/galaxyproject/galaxy/pull/17581 won't be available in Galaxy before 24.1 is released.
**Label maps** are images with pixel-level annotations, usually corresponding to distinct image regions (e.g., objects). We avoid the terms *label image* and *labeled image*, since these can be easily confused with image-level annotations (instead of pixel-level). The labels (pixel values) must uniquely identify the labeled image regions (i.e. labels must be unique, even for non-adjacent image regions). If a label semantically corresponds to the image background, that label should be 0.

**Binary images** are a special case of label maps with only two labels (e.g., image background and image foreground). To facilitate visual perception, the foreground label should correspond to white (value 255 for `uint8` images and value 65535 for `uint16` images), since background corresponds to the label 0, which is black.

**Intensity images** are images which are *not* label maps (and thus neither binary images).

## File types

If a tool wrapper only supports single-channel 2-D images and uses a Python script, the structure of the input should be verified right after loading the image:

```python
im = skimage.io.imread(args.input)
im = np.squeeze(im) # remove axes with length 1
assert im.ndim == 2
```

Tools with **label map inputs** should accept PNG and TIFF files. Tools with **label map outputs** should produce either `uint16` single-channel PNG or `uint16` single-channel TIFF. Using `uint8` instead of `uint16` is also acceptable, if there definetely are no more than 256 different labels. Using `uint8` should be preferred for binary images.

> [!NOTE]
> It is a common misconception that PNG files must be RGB or RGBA, and that only `uint8` pixel values are supported. For example, the `cv2` module (OpenCV) can be used to create single-channel PNG files, or PNG files with `uint16` pixel values. Such files can then be read by `skimage.io.imread` without issues (however, `skimage.io.imwrite` seems not to be able to write such PNG files).

Tools with **intensity image inputs** should accept PNG and TIFF files. Tools with **intensity image outputs** can be any data type and either PNG or TIFF. Image outputs meant for visualization (e.g., segmentation overlays, charts) should be PNG.

## Testing

### Testing infrastructure

The support for the new [`image_diff` output verification method](https://docs.galaxyproject.org/en/latest/dev/schema.html#tool-tests-test-output) and [assertions for image data](https://docs.galaxyproject.org/en/latest/dev/schema.html#assertions-for-image-data) for Galaxy tool testing probably won't be available in Galaxy before 24.1 is released.

Meanwhile, they are already available in the CI of the **galaxy-image-analyis** repostiroy! 🎉 https://github.com/BMCV/galaxy-image-analysis/pull/117

To also use them locally, you need to install the development versions of two Galaxy packages:
To also use them locally, you need to install the development versions of two Galaxy packages, pillow, and tifffile:
```python
python -m pip install git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/util
python -m pip install git+https://[email protected]/galaxyproject/galaxy.git@5c1d045ce7b1e45f85608346baed5455324ee967#subdirectory=packages/tool_util
python -m pip install git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/util
python -m pip install git+https://[email protected]/kostrykin/galaxy.git@galaxy-image-analysis#subdirectory=packages/tool_util
python -m pip install pillow tifffile
```

The commit [`5c1d045ce7b1e45f85608346baed5455324ee967`](https://github.com/galaxyproject/galaxy/commit/5c1d045ce7b1e45f85608346baed5455324ee967) corresponds to the latest merged bug fixes.
The [galaxy-image-analysis branch](https://github.com/kostrykin/galaxy/tree/galaxy-image-analysis) of the <https://github.com/kostrykin/galaxy> fork is the same as the [23.1 release of Galaxy](https://github.com/galaxyproject/galaxy/tree/release_23.1), plus the support for the image-based verification extensions.

In addition, instead of running `planemo test`, you should use:
```python
planemo test --galaxy_source https://github.com/kostrykin/galaxy --galaxy_branch galaxy-image-analysis
```
The [galaxy-image-analysis branch](https://github.com/kostrykin/galaxy/tree/galaxy-image-analysis) of the <https://github.com/kostrykin/galaxy> fork is the same as the [23.1 release of Galaxy](https://github.com/galaxyproject/galaxy/tree/release_23.1), plus the support for the image-based verification extensions.
Linting with `planemo lint` works as usual.

### Writing tests

We recommend using macros for verification of image outputs. The macros are loaded as follows:
```xml
<macros>
<import>tests.xml</import>
</macros>
```

For testing of **binary image outputs** we recommend using the `mae` metric (mean absolute error). With the default value of `eps` of 0.01, this asserts that at most 1% of the image pixels are labeled differently:
```xml
<expand macro="tests/binary_image_diff" name="output" value="output.tif" ftype="tiff"/>
```
The macro also ensures that the image contains two distinct label values, which are not interchangable.

For testing of non-binary **label map outputs** with interchangeable labels, we recommend using the `iou` metric (one minus the *intersection over the union*). With the default value of `eps` of 0.01, this asserts that there is no labeled image region with an *intersection over the union* of less than 99%:
```xml
<expand macro="tests/label_image_diff" name="output" value="output.tif" ftype="tiff"/>
```
Label 0 is commonly connotated as the image background, and is not interchangable by default. Use `pin_labels=""` to make it interchangable.

For testing of **intensity image outputs** we recommend the `rms` metric (root mean square), because it is very sensitive to large pixel value differences, but tolerates smaller differences:
```xml
<expand macro="tests/intensity_image_diff" name="output" value="output.tif" ftype="tiff"/>
```
For `uint8` and `uint16` images, increasing the default value of `eps` to `1.0` should be tolerable, if required.

## Future extensions

Below is a list of open questions:

- **How do we want to cope with multi-channel label maps?** For example, do or can we distinguish RGB labels from multi-channel binary masks, which are sometimes used to represent overlapping objects?

- How can we distinguish multi-channel 2-D images from single-channel 3-D images?

- How can we make clear to the user, whether a tool requires a 2-D image or also supports 3-D?
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ If Galaxy Image Analysis helped with the analysis of your data, please do not fo
- [Process images using arithmetic expressions](https://usegalaxy.eu/root?tool_id=toolshed.g2.bx.psu.edu/repos/imgteam/image_math/image_math) with NumPy
- [Scale image](https://usegalaxy.eu/root?tool_id=toolshed.g2.bx.psu.edu/repos/imgteam/scale_image/ip_scale_image) with scikit-image
- [Show image info](https://usegalaxy.eu/root?tool_id=toolshed.g2.bx.psu.edu/repos/imgteam/image_info/ip_imageinfo) with Bioformats
- [Slice image](https://usegalaxy.eu/root?tool_id=toolshed.g2.bx.psu.edu/repos/imgteam/slice_image/ip_slice_image)
- [Slice image into patches](https://usegalaxy.eu/root?tool_id=toolshed.g2.bx.psu.edu/repos/imgteam/slice_image/ip_slice_image)
- [Switch axis coordinates](https://usegalaxy.eu/root?tool_id=toolshed.g2.bx.psu.edu/repos/imgteam/imagecoordinates_flipaxis/imagecoordinates_flipaxis)

### Image conversion
Expand Down
23 changes: 23 additions & 0 deletions macros/creators.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<macros>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please note that the entire file is shipped to all tools.


<xml name="creators/bmcv">
<organization name="Biomedical Computer Vision Group, Heidelberg Universtiy" alternateName="BMCV" url="http://www.bioquant.uni-heidelberg.de/research/groups/biomedical_computer_vision.html" />
<yield />
</xml>

<xml name="creators/alliecreason">
<person givenName="Allison" familyName="Creason"/>
<yield/>
</xml>

<xml name="creators/bugraoezdemir">
<person givenName="Bugra" familyName="Oezdemir"/>
<yield/>
</xml>

<xml name="creators/thawn">
<person givenName="Till" familyName="Korten"/>
<yield/>
</xml>

</macros>
6 changes: 0 additions & 6 deletions macros/creators/bmcv.xml

This file was deleted.

95 changes: 95 additions & 0 deletions macros/tests.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
<macros>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this make it very unreadable for the actual tool?
And also here those macro files are shipped to all tools that import them, even if they do not use most of the stuff ... and it gets parsed and loaded into Galaxy I think.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm torn, I like how compact it is, but I'm worried those files are growing and growing and single tools don't use much of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the feedback!

On the other hand, if the macros were split into more fine-grained files, this would result in more I/O operations for loading the tools. Do you think that this is tolerable and parsing a few superfluous macros is worse?

My concern is due to: I remember from my C++ days, that, for compilers, I/O operations are the reason while compile times are long. By concatenating all files into a single file (aka unitybuild), compile times could be reduced by >90% (the improvement was somewhat less when using SSD instead of HDD but still huge).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand, but I guess this is not whats happening here. Every tool will ship the macro, so the number of files is the same in both cases. I one scenario its just a lot small files, and in your case its potentially a lot of big files with a lot of information that is not used for individual tools.

Its an academic discussion, please keep it like it is, I just wanted to point out this disadvantage.


<!-- Macros for verification of image outputs -->

<xml
name="tests/binary_image_diff"
tokens="name,value,ftype,metric,eps"
token_metric="mae"
token_eps="0.01">

<output name="@NAME@" value="@VALUE@" ftype="@FTYPE@" compare="image_diff" metric="@METRIC@" eps="@EPS@" pin_labels="0">
<assert_contents>
<has_image_n_labels n="2"/>
<yield/>
</assert_contents>
</output>

</xml>

<xml
name="tests/label_image_diff"
tokens="name,value,ftype,metric,eps,pin_labels"
token_metric="iou"
token_eps="0.01"
token_pin_labels="0">

<output name="@NAME@" value="@VALUE@" ftype="@FTYPE@" compare="image_diff" metric="@METRIC@" eps="@EPS@" pin_labels="@PIN_LABELS@">
<assert_contents>
<yield/>
</assert_contents>
</output>

</xml>

<xml
name="tests/intensity_image_diff"
tokens="name,value,ftype,metric,eps"
token_metric="rms"
token_eps="0.01">

<output name="@NAME@" value="@VALUE@" ftype="@FTYPE@" compare="image_diff" metric="@METRIC@" eps="@EPS@">
<assert_contents>
<yield/>
</assert_contents>
</output>

</xml>

<!-- Variants of the above for verification of collection elements -->

<xml
name="tests/binary_image_diff/element"
tokens="name,value,ftype,metric,eps"
token_metric="mae"
token_eps="0.01">

<element name="@NAME@" value="@VALUE@" ftype="@FTYPE@" compare="image_diff" metric="@METRIC@" eps="@EPS@" pin_labels="0">
<assert_contents>
<has_image_n_labels n="2"/>
<yield/>
</assert_contents>
</element>

</xml>

<xml
name="tests/label_image_diff/element"
tokens="name,value,ftype,metric,eps"
token_metric="iou"
token_eps="0.01"
token_pin_labels="0">

<element name="@NAME@" value="@VALUE@" ftype="@FTYPE@" compare="image_diff" metric="@METRIC@" eps="@EPS@" pin_labels="@PIN_LABELS@">
<assert_contents>
<yield/>
</assert_contents>
</element>

</xml>

<xml
name="tests/intensity_image_diff/element"
tokens="name,value,ftype,metric,eps"
token_metric="rms"
token_eps="0.01">

<element name="@NAME@" value="@VALUE@" ftype="@FTYPE@" compare="image_diff" metric="@METRIC@" eps="@EPS@">
<assert_contents>
<yield/>
</assert_contents>
</element>

</xml>

</macros>
Loading