Skip to content

Conversation

@Gurpreet-Ghattaoraya
Copy link

@Gurpreet-Ghattaoraya Gurpreet-Ghattaoraya commented Jul 14, 2022

This datacheck is intented to make sure that, for human DBs, that the ancestral allele pipeline has ran - the pipeline populates the ancestral allele information for COSMIC and ClinVar variant sources. So the check makes sure these columns have data.

Copy link
Contributor

@marcoooo marcoooo left a comment

Choose a reason for hiding this comment

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

Can you please update the description to give me more context about this PR?
Check that you tested it on a sample db etc..
Thanks!

@dglemos dglemos self-requested a review July 14, 2022 15:24
@Gurpreet-Ghattaoraya
Copy link
Author

Hi @marcoooo - I have updated the description and I have asked @dglemos to review on the code on the Variation team side of things. I have tested the code against a few DBs - I can provide a link to the Jira ticket which show what I have tested

This is my first contribution to the Datachecks so I am still learning. Please let me know if I have missed anything else out?

@marcoooo marcoooo self-assigned this Jul 14, 2022
@marcoooo marcoooo added this to the Ensembl 109 milestone Jul 14, 2022
@marcoooo
Copy link
Contributor

Hi @marcoooo - I have updated the description and I have asked @dglemos to review on the code on the Variation team side of things. I have tested the code against a few DBs - I can provide a link to the Jira ticket which show what I have tested

This is my first contribution to the Datachecks so I am still learning. Please let me know if I have missed anything else out?

Thanks! Quick question, is it expected to be run for current release 108? If so, the PR should be opened against release/108 branch.

@coveralls
Copy link

coveralls commented Jul 14, 2022

Pull Request Test Coverage Report for Build 2199

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 98.705%

Totals Coverage Status
Change from base Build 2193: 0.004%
Covered Lines: 2363
Relevant Lines: 2394

💛 - Coveralls

@Gurpreet-Ghattaoraya
Copy link
Author

Yes, we do want to run it on 108. I'll edit the pull request to point towards 108 instead.

@Gurpreet-Ghattaoraya Gurpreet-Ghattaoraya changed the base branch from main to release/108 July 14, 2022 16:09
@marcoooo
Copy link
Contributor

Yes, we do want to run it on 108. I'll edit the pull request to point towards 108 instead.

I am afraid your PR now include unwanted/untested commit. You might have to redo the updates from a fresh branch issued from current release/108

@Gurpreet-Ghattaoraya
Copy link
Author

Yes, we do want to run it on 108. I'll edit the pull request to point towards 108 instead.

I am afraid your PR now include unwanted/untested commit. You might have to redo the updates from a fresh branch issued from current release/108

Yes sorry, I had originally creted my branch from main - I'll try again but using release/108.

Copy link
Contributor

@marcoooo marcoooo left a comment

Choose a reason for hiding this comment

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

one general remarks, if possible, it's best to avoid direct SQL in DC, best to use the related API, because the day the schema changes... we can forget to update related DC.

@Gurpreet-Ghattaoraya
Copy link
Author

one general remarks, if possible, it's best to avoid direct SQL in DC, best to use the related API, because the day the schema changes... we can forget to update related DC.

Can I clarify please? By "direct SQL" does that mean my example line of code
my $is_COSMIC = $self->dba->dbc->sql_helper->execute(-SQL => $sql_COSMIC_exists); ?

@marcoooo
Copy link
Contributor

one general remarks, if possible, it's best to avoid direct SQL in DC, best to use the related API, because the day the schema changes... we can forget to update related DC.

Can I clarify please? By "direct SQL" does that mean my example line of code my $is_COSMIC = $self->dba->dbc->sql_helper->execute(-SQL => $sql_COSMIC_exists); ?

That means to use the Variation Perl API where possible.

@marcoooo marcoooo merged commit ff36faa into Ensembl:release/108 Jul 15, 2022
@Gurpreet-Ghattaoraya Gurpreet-Ghattaoraya deleted the ENSVAR-4956 branch July 18, 2022 08:58
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.

3 participants