-
Notifications
You must be signed in to change notification settings - Fork 34
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
Australian mouse brain #409
base: main
Are you sure you want to change the base?
Conversation
Thanks @PolarBean! Bear with us, it might be a few days until we can review this. |
Thanks @PolarBean - I will run it locally this week, and upload it to GIN assuming I get the validation to pass too :) |
There was a problem hiding this 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 @PolarBean - so much effort!
I've run this locally, and it's largely looking very good. I think there's two things we need to sort out with the annotations (at least for the result I get):
- in the annotation image, there are some spurious pixels (e.g. on coronal slice 9238) outside the brain that claim to be part of CA1-Pyramidal oriens layer!
image - it looks like the annotations file does not have pixels labelled with [35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 62, 64, 256, 257, 258, 259, 260, 261, 262] but somehow these make their way into the metadata, leading to the validation complaining about these regions not having meshes associated with them. Is this expected?
I've also made some tiny nitpicky comments about the code.
brainglobe_atlasapi/atlas_generation/atlas_scripts/australian_mouse_brain.py
Outdated
Show resolved
Hide resolved
brainglobe_atlasapi/atlas_generation/atlas_scripts/australian_mouse_brain.py
Outdated
Show resolved
Hide resolved
brainglobe_atlasapi/atlas_generation/atlas_scripts/australian_mouse_brain.py
Outdated
Show resolved
Hide resolved
brainglobe_atlasapi/atlas_generation/atlas_scripts/australian_mouse_brain.py
Outdated
Show resolved
Hide resolved
These are in the atlas file shared by the creator so I would say leave them (even though you are correct they are a mistake). The only other alternative I can think of is brainglobe having an automated filter for out of brain pixels. But then we are getting into the territory of creating a brainglobe version of each atlas which is unique to brainglobe. I'm not fussed which way we choose but we should be consistent.
Yeah this is expected :) For instance you have layers 1, 2, 3 of some structure but only layers 1 and 3 are actually delineated. 2 can still exist in the metadata as it should exist just hasn't been delineated yet. If we want to create a rule that these should be filtered out then we would run into the same issue described above. Ill go through the code comments now! |
…mouse_brain.py Co-authored-by: Alessandro Felder <[email protected]>
…mouse_brain.py Co-authored-by: Alessandro Felder <[email protected]>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Previously we decided (and should document) that in these cases we would release a version without “editing” anything, and then a later version with the corrections. Not sure we’ve done this yet, but if we do we should make sure it’s clearly documented. |
Happy to be outvoted, but I think in this example, 2 shouldn’t be in the metadata, as it doesn’t actually exist in the atlas. This would be consistent with other BG atlases anyway. |
I disagree, as the ontology itself is valuable. If someone creates a highly detailed ontology, even if it has not yet been fully delineated, I would like to access that through brainglobe. |
I can see why it would be useful, but to add that information, we would need to change the API. Currently third party tools rely on e.g. a 1-2-1 mapping between region in the ontology and a mesh. I've raised an issue to track this (#439) |
Great! For now then I'll remove the regions listed as not existing in the volume :) |
Thanks!!! Let me know when you've done so - then we can merge this 🎉 and I'll upload to GIN. |
This is to add the australian mouse brain atlas, resolving this issue.
#319
The benefits of this atlas are that it is a 15µm resolution MRI template. Which I believe is currently the highest resolution MRI template by quite a distance (Though they used a novel method to upsample from 30µm described here: https://cds.ismrm.org/protected/12MProceedings/PDFfiles/1077.pdf.)
Also these annotations are at least partially based on the Paxinos ontology and Paxinos is an author which may be useful for comparative purposes as the Paxinos atlas is widely used for 2D atlasing.
The challenges when adding this atlas that I faced were the following:
Nonetheless, hopefully this is a useful addition to the API.