Skip to content
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

Conversation

beatrizsaldana
Copy link
Member

AG-863: Update gene_info processing for druggability revamp

Changes

  • Removed old druggability input file (syn13363443.11) from gene_info files in config and test_config
  • Removed old druggability input file (syn13363443.11) from gene_info provenance in config and test_config
  • Updated gene_metadata version number from 13 to 14 in config and test_config
  • Used new druggability source file (syn64123611.1) in config, call it pharos_classes
  • Updated druggability custom transform to keep only the pharos_class object
  • Updated gene_info druggability tests and testing assets
  • Updated druggability gx validation

Acceptance Criteria

  • ADT configs incorporate the appropriate source file/provenance updates
  • transform_gene_info is updated as required to produce the target data structure
  • custom transform tests are added/updated as required
  • GX validation is adjusted as required

@BWMac
Copy link
Contributor

BWMac commented Dec 9, 2024

@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.

Copy link
Contributor

@BWMac BWMac left a 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.

Copy link
Contributor

@BWMac BWMac left a 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.

@beatrizsaldana
Copy link
Member Author

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.

@BWMac according to @JessterB the field does need to be nested.

Copy link
Contributor

@JessterB JessterB left a 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 ;)

@BWMac BWMac self-requested a review December 11, 2024 22:34
Copy link
Contributor

@BWMac BWMac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

Copy link
Contributor

@jaclynbeck-sage jaclynbeck-sage left a 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

@beatrizsaldana beatrizsaldana merged commit 5c5931b into dev Dec 11, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants