-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update cap_anndata_schema.md #118
Conversation
docs/cap_anndata_schema.md
Outdated
@@ -1370,12 +1370,12 @@ Key-value pair in the `uns` dictionary | |||
Python dictionary within the `uns` dictionary, with the key the string `[cellannotation_setname]` | |||
|
|||
|
|||
#### [cellannotation_setname]--metadata | |||
#### [cellannotation_setname]_metadata |
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.
we don't have the [cellannotation_setname]_metadata
field in the schema.
uns consists of constant name cellannotation_metadata
which will maps to dict whith [cellannotation_setname]
keys.
So the right example is:
uns = {
"title": blabla,
"cellannotation_metadata": {
"cell_type": {"description": blabla, "annotation_method": blabla...},
"author_cell_type":{"description": blabla, "annotation_method": blabla...},
}
}
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.
Excellent point, I updated [cellannotation_setname]_metadata
to cellannotation_metadata
but used Ugur's suggestion to have the example value be the value only and not include the key
docs/cap_anndata_schema.md
Outdated
@@ -1370,12 +1370,12 @@ Key-value pair in the `uns` dictionary | |||
Python dictionary within the `uns` dictionary, with the key the string `[cellannotation_setname]` | |||
|
|||
|
|||
#### [cellannotation_setname]--metadata | |||
#### [cellannotation_setname]_metadata |
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.
this should change to cellannotation_metadata
right?
#### [cellannotation_setname]_metadata | |
#### [cellannotation_metadata]_metadata |
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.
@ubyndr it should be [cellannotation_metadata]
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.
Might be confusion above
[cellannotation_setname]
is the name of the "labelset" or cell annotation set. e.g. cell_type
I am guessing the misalignment is that Roman + Evan simplified this earlier. That example is correct.
• We tag all of this uns
stuff as cellannotation_metadata
.
• Keys within that dictionary are the set names themselves i.e. [cellannotation_setname]
Are we aligned now?
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 suggested two possible changes for your review.
Co-authored-by: Ismail Ugur Bayindir <ubyndr@gmail.com>
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.
LGTM
@evanbiederstedt - Did this just change the schema so that it now differs from the one you are actually using on CAP? I want something for now that will conform to what CAP currently does so that we can code to it it load data. If you are changing the key name, then (a) please delay until implemented in CAP (b) please, please, please change this to (Also worth noting that the problems here are a good illustration of why programatically validateable schemas are good) |
For MVP-5404
Made the following two updates:
[cellannotation_setname]_metadata
[cellannotation_setname]--metadata
to[cellannotation_setname]_metadata