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
Translator for clinicaltrials.gov #2153
Conversation
@bwiernik and @adam3smith any input on what item type to use for clinical trials? |
I would use Report for this. |
@dhimmel and @agitter two quick questions that I thought you may have answers to:
I looked https://www.who.int/ictrp/How_to_cite.pdf and https://blogs.uoregon.edu/annie/2017/10/25/clinical-trial-apa-format/ but they didn't seem to be conclusive. |
For things like preprints, Zotero translators typically save the last updated date (i.e., the date of the version of the item actually being viewed) as the date. The first submit date could be stored in Extra with the label "Original date:" |
Agree on the regular date, but I'd be careful with using |
That's a really good point. "Submitted" might be a better (and rarely used in citation styles) variable. |
Just looking at a random record "SponsorCollaboratorsModule":{
"ResponsibleParty":{
"ResponsiblePartyType":"Principal Investigator",
"ResponsiblePartyInvestigatorFullName":"Chen Xiaoping",
"ResponsiblePartyInvestigatorTitle":"Principal Investigator",
"ResponsiblePartyInvestigatorAffiliation":"Tongji Hospital"
},
"LeadSponsor":{
"LeadSponsorName":"Tongji Hospital",
"LeadSponsorClass":"OTHER"
}
}, Some documentationFrom https://prsinfo.clinicaltrials.gov/definitions.html:
I think perhaps we want everything: the lead investigator, the sponsor, and collaborators. Each one of these could be different authors. I don't know too much about clinical trials however, so would be interested in what others think |
Thank you all for the quick responses. Much appreciated! My last commit attempts to incorporate that feedback.
|
When I run the tests locally it says that the
i.e. it's getting @adam3smith or someone else experienced with Zotero, any input on this? |
Haven't looked at your code, but |
Thanks @adam3smith! That worked. All three test are now passing. Ready for review. |
A few immediate comments:
|
Thank you for the review @bwiernik! I've addressed everything except point 2. I think your recommendation makes sense just one point of clarification: We would store the sponsor in extra as "Sponsor: " no matter what, even if there was no principle investigator and the sponsor was also listed as the author? Pro: You could tell if the sponsor was listed as the author. (if clinical trials always have a sponsor than this is negated as you could just check to see if "Sponsor: " in extra was missing and if so assume know that the "author" was actually the "Sponsor") |
It would be fairly clear that the sponsor was listed as an author by virtue of it being an organizational (single-field) author. Another option rather than placing in Extra at all would be to include it as a "Contributor" or "Author" depending on whether there was a personal author (PI). |
@bwiernik That makes sense to me. I implemented that and we can see if anyone has other opinions on how it should be handled. Thanks again for looking at this! |
Just wanted to check in here. @rdvelazquez are you good on your end and just waiting for review? |
Yep. Things are good on my end and just waiting for review. |
@bwiernik / @adam3smith any chance either you have time to re-review? Would be greatly appreciated. |
Just checking on the status of getting this reviewed and incorporated into Zotero. This would be helpful for the https://github.com/greenelab/covid19-review project. |
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.
Sorry for the delay -- difficult to keep up with these during wfh. A couple of questions and comments -- I'll try to be quick to get back to you once you reply
clinicaltrials.gov.js
Outdated
*/ | ||
|
||
function detectWeb(doc, url) { | ||
if (url.includes("https://clinicaltrials.gov/ct2/results")) { |
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.
can we not make multiples work or have you just not gotten to it?
We strongly prefer having multiples available -- especially for things like systematic reviews this would seems super useful.
In any case, we should then just exclude them via target regex - and make the target regex as well as detectWeb()
more restrictive overall: in its current form the translator would detect e.g. the homepage and help pages as reports and then fail on import. This should never happen.
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 made the target more restrictive. I wasn't sure if any additional checks should go in detectWeb()
; it seemed like the target was the preferred place to make it more restrictive.
I thought the single case was more important than the multiples and wanted to focus my limited time on getting that done first. I know we need the single use case for the active https://github.com/greenelab/covid19-review and wasn't aware of a use case for the multiples (I assume this would be getting all the citations for what the clinicalTrials website returns from a search?). I'd be willing to help get that incorporated but I'm not sure how much time I'll have in the next few weeks.
clinicaltrials.gov.js
Outdated
return dateTime.split(" ")[0].split(":").join("-"); | ||
} | ||
|
||
function nameToFirstAndLast(rawName) { |
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.
Could you add a comment on what exactly this does, ideally with a data example. If I'm reading this correctly, this would also remove authors initials/second names which doesn't seem ideal. Note that Zotero has a built-in ZU.cleanAuthor
function that might be helpful here depending on the format.
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 felt like I was likely reinventing the wheel when I wrote that function but couldn't find a good replacement. Thanks for pointing out ZU.cleanAuthor
that was just what I needed!
clinicaltrials.gov.js
Outdated
"shortTitle": "Study to Evaluate the Safety and Antiviral Activity of Remdesivir (GS-5734™) in Participants With Severe Coronavirus Disease (COVID-19)", | ||
"url": "https://clinicaltrials.gov/ct2/show/NCT04292899", | ||
"institution": "clinicaltrials.gov", | ||
"reportNumber": "NCT04292899", |
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.
are the test generated with scaffold? It's odd that the indenting is off for report details. Let's fix this
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 fixed the indenting issue. I generated the tests myself without using scaffold; I think I tried using it but had some issues getting it running if I remember correctly.
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.
Just to clarify, Scaffold is now just Tools → Developer → Translator Editor in Zotero, not a separate plugin. The test needs to be generated by Scaffold to be valid. (If you generated it by hand, how did you test it?)
If you're having trouble with something in Scaffold, let us know.
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 clarifying that @dstillman. That's how I tested it (and now that I'm looking at it again I can see that it says "Scaffold" at the top of the window... ). That was very helpful for developing the translator and how I tested it (I also used a simple node.js script to test the parsing while developing it so that I could get more instantaneous feedback)
test fails, so not adding, but tested successfully in Firefox
(sorry for the linting problems -- linting is a mess on Windows, no idea how people do this. Will fix asap or feel free to fix on your end) |
@adam3smith Thanks so much for the edits! I fixed the listing issues by running |
Terrific, thanks -- this looks great! |
using report item type for now; test for multiple fails, so not adding, but tested successfully in Firefox
TODO:
Currently usingusingjournalArticle
but there have been discussions of usingdataset
or creating a new typereport
as recommended by @bwwiernikGot it set up and everything works except for an issue with matching the "extra" fieldAll three tests are passing nowImplement the search (if we think its a feature this should have)I don't see the need for being able to cite all the trials from a particular search of clinicaltrials.gov at this point (could always be added in later if needed,item parsing and other TODOstranslatorType
andbrowserSupport
in the metadata I think these are correct; just following these docscloses #1952
relates to manubot/manubot#216