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

fix hemispheres being cut slightly off-centred #454

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

carlocastoldi
Copy link
Contributor

@carlocastoldi carlocastoldi commented Jan 29, 2025

This fixes a small problem with ~symmetric atlases with an even-sized frontal axis to be cut slightly off-centred.

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
This produces

What does this PR do?

References

Please reference any existing issues/PRs that relate to this PR.

How has this PR been tested?

import numpy as np
from brainglobe_atlasapi import BrainGlobeAtlas

atlas = BrainGlobeAtlas(atlas_name="allen_mouse_10um")
round(atlas.metadata["shape"][2] / 2)
all(atlas.hemispheres[0,0,571:] == 1)
all(atlas.hemispheres[0,0,:571] == 2)

last two commands should be True only with 570, not with 571 as the splitting axis depth.

Additionally, this is also shown in ABBA, as atlas annotations imported with the auxilium of BrainGlobe are offcentred:

image

From the image, you can notice that BrainGlobe's (yellow) and ABBA's ASR annotations fully overlap, however BrainGlobe splits the hemispheres slightly on the right.

P.S. the left annotations are from ABBA's RSA atlas. For some reason the transformation shifted it one unit (µm). Dunno the reason, but that's not the point.

this is the result with this PR:

image

Is this a breaking change?

Not that I am aware of.

Does this PR require an update to the documentation?

No

Checklist:

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

@alessandrofelder
Copy link
Member

Thanks @carlocastoldi - looks good at first glance. Will have a slightly deeper look tomorrow.

@alessandrofelder alessandrofelder self-requested a review January 29, 2025 17:48
Copy link
Member

@alessandrofelder alessandrofelder left a comment

Choose a reason for hiding this comment

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

I have confirmed the bug in napari on the allen_mouse_10um atlas. Here is a closeup of the midsagittal plane in a coronal section:

image

One can also be convinced of its presence by examining the values below in the Allen Mouse 10um atlas:

In [48]: atlas.shape[2]
Out[48]: 1140

In [49]: (atlas.hemispheres==2).sum()-(atlas.hemispheres==1).sum()
Out[49]: np.int64(2112000)

In [50]: atlas.shape[0]*atlas.shape[1]*2
Out[50]: 2112000

Which shows that the number of pixels in one hemisphere differs by 2x number of pixels in a sagittal plane from the number of pixels in the other hemisphere. This is despite the number of LR pixels being even (so there should be no difference).

@alessandrofelder alessandrofelder merged commit 040c450 into brainglobe:main Jan 30, 2025
14 checks passed
@alessandrofelder
Copy link
Member

Thanks a lot for fixing @carlocastoldi 🙌

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