-
Notifications
You must be signed in to change notification settings - Fork 3
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 gene info processing for druggability revamp #163
Update gene info processing for druggability revamp #163
Conversation
…clude pharos_class in druggability object
…idation itself so I can run adt without problems and then update gx validation
@beatrizsaldana I think your CI is failing because of the schema update I did to the GX tables as part of #161. If you merge those changes into your branch it should work again. |
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.
My main question which could have some implications for the shape of this PR is: Does the druggability
field need to be nested anymore? If not, this can be simplified quite a bit.
src/agoradatatools/great_expectations/gx/expectations/gene_info.json
Outdated
Show resolved
Hide resolved
src/agoradatatools/great_expectations/gx/expectations/gene_info.json
Outdated
Show resolved
Hide resolved
src/agoradatatools/great_expectations/gx/json_schemas/gene_info/druggability.json
Show resolved
Hide resolved
src/agoradatatools/great_expectations/gx/json_schemas/gene_info/druggability.json
Outdated
Show resolved
Hide resolved
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.
My main question which could have some implications for the shape of this PR is: Does the druggability
field need to be nested anymore? If not, this can be simplified quite a bit.
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! (assuming CI passes ;)
Quality Gate passedIssues Measures |
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.
🚀
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 great, assuming CI passes w/ Brad's update
AG-863: Update gene_info processing for druggability revamp
Changes
pharos_classes
Acceptance Criteria