-
Notifications
You must be signed in to change notification settings - Fork 11
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 Swanson atlas v3 #30
Conversation
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.
The schemes looks good and are in accordance with what we agreed upon.
A minor question, the name of the parcellation entities and parcellation entity versions all start with a lower case letter, is this a general practice?
The reason for this question is that there is a general practice to start white matter name with a lower case letter and grey matter names with a capital letter.
@UlrikeS91 thank you for this huge PR. Here some smaller issues: Correction for naming conventions (sorry for the missing documentation): CCS/CCSV (correct refs of atids respectively in BA/BAV) BA/BAV (correct refs of atids respectively in PE/PEV) ISBN / Webresources: I think that's all :) |
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.
cf latest comment
The feedback from both @aeidi89 and @lzehl have been integrated. I also made an overview, the script and the input files available here: https://github.com/UlrikeS91/AtlasIntegration-into-openMINDS/tree/main/Swanson-BrainMaps. |
Found syntax error and some typos
@UlrikeS91 for the codespell errors. Could you double check if these are spelling errors or just exceptions we don't yet catch in the codespell? Here are the files / errors to check:
|
Thanks! These are correct. They are the abbreviations of the brain regions and, luckily, no typos 🙂 |
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.
went through one of each instance type. looks good. parent structures of PEs and PEVs are still missing (as discussed as far as I remember).
@UlrikeS91 thanks a lot!
@lzehl and @aeidi89
A rough summary of the content:
Brain atlas
Swanson refers to his atlas as "Brain Maps Atlases" when he talks about 1st - 4th edition. So, we went with this.
fullName: Swanson's Brain Maps Atlases
no shortName (we couldn't come up with anything shorter than the full name without ending up at the abbreviation)
abbreviation: SwansonBMA
Brain atlas version
Swanson's atlas is called "Brain maps: structure of the rat brain". That's obviously the full name. And he uses BM as his abbreviation. As discusse with @lzehl, I added "Swanson's"/"Swanson" to all of it.
fullName: Swanson's Brain maps: structure of the rat brain
shortName: Swanson's Brain maps
abbreviation: SwansonBM
versionID: 3rd ed.
Common coordinate space
As discussed with @lzehl, we went with "Swanson's Stereotactic Brain". This is entirely made up by us.
fullName: "Swanson's Stereotactic Brain"
Again no shortName
abbreviation: "SwansonSB"
Common coordinate space version
This turned out a bit different then discussed. We somehow assumed that he also used Interaural as origin like Paxinos and Watson since he based his coordinate system on them, but that was not the case. So, there is only one space where the origin is bregma. He does have a physical space as well, but we do not register this - at least not in this round.
no fullName (we tried to come up with something but nothing sounded right, we don't feel that the short name made sense as a full name, so we skipped it)
shortName: Swanson's Stereotactic Brain (Bregma)
abbreviation: SwansonSB-Bregma
versionID: 1st ed.
ISBN and webResource
I had to create instances folder for webResource and ISBN to be able to add them to the atlas. The same webResource has been used for both the fullDocumentation of the BAV and CCSV. Note: The webResource leads to https://larryswanson.com/?page_id=164. This is also the homepage of the BAV (but there it is obviously registered as an IRI directly).
Parcellation entity (versions)
There should be 1159 for each and a few regions have been edited manually based on errors we found and the errata that Swansons published alongside the atlas version (https://larrywswanson.com/wp-content/uploads/2015/03/BM3-Errata.pdf).
I made similar adjustments to the corresponding PEs. There we don't have additionalRemarks, so I couldn't describe the change, but this should not be a problem since they are linked to each other. I did add the alternateNames there, too.
@aeidi89 please have a look at these files and the overall content of the jsonlds to make sure that I generated the files in accordance to what we discussed.
@lzehl could you please review this PR with special focus on syntax, naming and organisation? You absolutely don't have to focus on the content in detail, just the overall structure and if there are obvious flaws in the content that you can spot without knowing the atlas itself. Please have a close look at the file names and their paths. I think I managed to follow the new convention, but this is the first time I'm following this new convention. There might be errors 😉