-
Notifications
You must be signed in to change notification settings - Fork 51
Add dorr_mouse_mri atlas #657
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
Conversation
|
I think you're working off of an old version of main, could you pull the most recent version of main and rebase or merge it into this branch? We're now storing atlas generation scripts in a separate directory in the root of the repository, and the |
|
Thanks @IgorTatarnikov - explains why pre-commit is working now! I've just pushed again. |
IgorTatarnikov
left a comment
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.
I've tried the packaging and it works great! I've left a few comments in terms of code style.
One outstanding question is how to handle L/R labels. BrainGlobe doesn't typically use separate annotation labels for L/R of the same structure, we use the hemisphere array to differentiate between left and right. @adamltyson @alessandrofelder do you know how we've handled atlases with separate left/right annotations in the past?
|
Thank you @IgorTatarnikov @adamltyson @alessandrofelder! Just pushed with the merged annotations and other formatting fixes. Used the same approach as the axolotl atlas. |
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 for making the changes.
I've generated the atlas and it looks great!
However, there are duplications in the region abbreviation names. For example, both the Hippocampus and the Hypothalamus are abbreviated as H, Medulla and Midbrain as M.
Not sure what's best to do in this case, you could use the a similar approach as you've done now (first capital letters of all regions) with a fallback to make sure there's at least 3 letters in the acronym, or keep track of acronyms already used and add a letter if it's not unique 🤔 You could also use the Allen mouse ontology as a guide but that may require some fuzzy matching, as not all regions described in this atlas have an equivalently named structure in the Allen.
|
Ah I see, my bad! Let me change that to avoid duplicates :) |
IgorTatarnikov
left a comment
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.
Looks great! I think this is good to go. I'll upload the packaged atlas to GIN and update the last_versions.conf file.
The next steps are:
- Update the table on the website, and the atlas details page.
- Add a quick blog post about the new atlas
- Update the README.md for this repo
We can open up issues in the appropriate repos to track the outstanding tasks.

Before submitting a pull request (PR), please read the contributing guide.
Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)
Description
What is this PR
What does this PR do?
Add Dorr mouse MRI atlas to Brainglobe API.
Does this PR require an update to the documentation?
Yes.
Checklist: