-
Notifications
You must be signed in to change notification settings - Fork 1
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
Additional curation features to facilitate imports into the Curation Workflow #34
Conversation
32425e1
to
05e9908
Compare
05e9908
to
d205300
Compare
be8682e
to
8709ef7
Compare
33dbd8f
to
934f124
Compare
32e1325
to
56dba01
Compare
56dba01
to
5b447ee
Compare
28300bf
to
9aacd52
Compare
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.
Only two comments. The one about including a record that identifies the curator should I suppose best be handled by breaking the issue out from here.
The second is about an inconsistency, and even if it's only fixing the HTML-commented text (in case I'm missing something in my comment), something should be done about it.
@@ -810,14 +810,17 @@ | |||
{ | |||
"label": "Laurasiarana", | |||
"cladeDefinition": "Laurasiarana, new clade name. Definition: The clade stemming from the most recent common ancestor of Rana temporaria Linne 1758 and Rana aurora Baird and Girard 1852.", | |||
"curatorComments": "The Laurasiarana expected node is not found in the TreeBase tree, so I've placed it where I think it should go.", |
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 don't think we currently have fields for identifying the curator (remind me if this is not true), but this comment reminds me that we do need to add this, at least in the form of name and ID, the latter presumably being an ORCID.
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.
Good point! I've added this as issue #59.
index.html
Outdated
manually added to the PHYX file using the "Display as JSON" | ||
option. | ||
--> | ||
<label for="testcase-id" class="col-md-1 control-label">Testcase IRI</label> |
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.
So it seems that we have a whole code block here to deal with a property we don't actually want to be used. If we had a large legacy of PHYX files, this may well be justified, but we don't. If this property is present in some files previously created, can't we just fix those files, especially given that nothing is in production yet anyway?
Thanks for the comments, Hilmar! In removing the |
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 good!
This pull request changes some aspects of the PHYX model in order to facilitate curation and to keep the Curation Tool in sync with the Curation Workflow. These changes includes: - Removed `@id` field (closes #2). - Added a verbatim specifier field for all specifiers (closes #17). - Added a curator comments field for all phyloreferences (closes #24). - Allows the curator to document that they expect a single node to resolve to multiple phyloreferences by selecting a matching labeled node from the phyloreferencing panel (#32). - Added a simple system for marking specifiers that could not be matched (#49). - Improved display of scientific names and specimens as specifiers.
This pull request changes some aspects of the PHYX model in order to facilitate curation and to keep the Curation Tool in sync with the Curation Workflow. These changes includes:
@id
field (closes Remove 'ID' from study metadata #2).This pull request is ready to be merged.