-
Notifications
You must be signed in to change notification settings - Fork 4
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
fix: update grouper API #433
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.
lgtm.
Thank you for writing up the from scratch data load in the README
normalized_id = normalized_drug[@descriptor_name][@id_name] | ||
create_new_drug(normalized_drug[@descriptor_name]) if Drug.find_by(concept_id: normalized_id).nil? | ||
normalized_id = normalized_drug['normalized_id'] | ||
create_new_drug(normalized_drug['therapeutic_agent'], normalized_id) if Drug.find_by(concept_id: normalized_id).nil? |
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.
potentially dumb Q, but why the change to a hard coded 'therapeutic_agent' tag. Was the descriptor always therapeutic agent anyways?
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 relates to some VA/VRS/GKS stuff introduced in the last few months. Previously, we were delivering VRSATILE therapeutic descriptors, but VRSATILE has been canned and therapeutics are a first class entity in the GA4GH knowledge model, e.g. described here: https://github.com/ga4gh/vrs-python/blob/1c9e4ab8d66c652a8d27430a77c4ad15834aa1ea/src/ga4gh/core/_internal/models.py#L229
The reason for instance variable vs hard coded string was there were some other methods that were able to be neutral with respect to drug versus gene if the descriptor_name
instance var was available, but they kind of lost their utility over time and I think it's easier just to hard code things now
close #428