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 cavefish atlas 🐟 #277

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add cavefish atlas 🐟 #277

wants to merge 9 commits into from

Conversation

alessandrofelder
Copy link
Member

@alessandrofelder alessandrofelder commented May 3, 2024

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

We'd like to support @Robkozol 's cavefish atlas.

What does this PR do?
Adds a packaging script for the cavefish atlas.

Some notes:

  • I've downsampled everything to 2x2x2 micron, so we don't have to download GBs of data through the atlas API - is that reasonable? (the alternative is to keep the atlas anisotropic, and upsample the isotropic annotations instead, I guess?)
  • I noticed the original reference images have slightly different dimensions:
(211, 1024, 1948) SPF2_cartpt_ref
(211, 1024, 1946) SPF2_terk_ref

For now, I've ever so slightly fudged the downsampling factors to make them both match the (isotropic) annotation image dimensions exactly.

This is what it looks like currently in grid view (row-wise from top left: root mesh, Pons mesh, Prepontine mesh; annotations, main (terk) reference, additional (cartpt) reference)
image

It passes our general internal validation for atlases.

References

Closes #259

How has this PR been tested?

Visual inspection and running atlas validation locally.

Is this a breaking change?

No

Does this PR require an update to the documentation?

We should add the cavefish atlas to the list of available atlases on the website (brainglobe/brainglobe.github.io#192)

Checklist:

  • The code has been tested locally
  • [n/a] Tests have been added to cover all new functionality (unit & integration)
  • [n/a] The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@alessandrofelder alessandrofelder self-assigned this May 3, 2024
@alessandrofelder alessandrofelder mentioned this pull request May 3, 2024
7 tasks
Robkozol and others added 7 commits May 23, 2024 15:20
tweaks:
* renamed some variables
* use pooch to unzip
* make root mesh white
* add additional reference
* downsample both references to match annotation (with slight fudge)
* use new URL
* remove broken parallelisation part
* make additional reference uint16
* make annotations uint8
* resolution now isotropic 2x2x2 micron
Copy link
Member

@adamltyson adamltyson left a comment

Choose a reason for hiding this comment

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

Looks good, but I'm very keen that we sort out this:

For now, I've ever so slightly fudged the downsampling factors to make them both match the (isotropic) annotation image dimensions exactly.

I made a few other comments to aid the future standardisation.

Comment on lines +115 to +117
reference_volume = zoom(
reference_volume, (1, 1.0 / 3.995, 1.0 / 3.995), order=0
) # 1/3.995 is a fudge to match annotation dimensions
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to get this clarified so if we're going to fudge something, we know exactly why.

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.

[Feature] Adding a cave fish atlas
4 participants