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

Feature/devsu 2518 add multivariant statement loading #405

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

elewis2
Copy link
Collaborator

@elewis2 elewis2 commented Nov 22, 2024

Adds report uploading functionality: report json can now include a section 'kbStatements', each of which can contain an array of kbmatches. During loading associations are created between each match in this array, and the parent statement.

Prevents associations being created between kbmatches and kbstatements that are not explicitly associated, in the kbmatches section.

…feature/DEVSU-2518-add-multivariant-statement-loading
Copy link

Unit Test Results

    1 files  ±0    62 suites  ±0   2m 45s ⏱️ ±0s
564 tests +2  564 ✔️ +2  0 💤 ±0  0 ❌ ±0 
561 runs  +2  561 ✔️ +2  0 💤 ±0  0 ❌ ±0 

Results for commit 1bd5454. ± Comparison against base commit f0d452f.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 91.11111% with 4 lines in your changes missing coverage. Please review.

Project coverage is 76.96%. Comparing base (f0d452f) to head (1bd5454).

Files with missing lines Patch % Lines
app/libs/createReport.js 88.88% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #405      +/-   ##
===========================================
+ Coverage    76.89%   76.96%   +0.07%     
===========================================
  Files          179      180       +1     
  Lines         6076     6113      +37     
  Branches       725      734       +9     
===========================================
+ Hits          4672     4705      +33     
- Misses        1328     1331       +3     
- Partials        76       77       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

}
]
}
],
"kbMatches": [
Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that each kbMatches record is a 1-1-1 relationship between the observed variant (variant & variantType), a gkb statement (kbStatementId), and the variant it match to in that statement's conditions (kbVariantId). So 1 record for each of these combinations, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At the moment it's possible to do a 1-many relationship with one kbmatch to many kbstatements, in this section. That functionality is not removed in this pr, it just adds the ability to load in the inverse way (1-many with one kbstatement to many kbmatches).

Copy link
Member

Choose a reason for hiding this comment

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

I meant for a sigle kbMatches record. Are you talking about the capability to create multiple records using the same observed variant but linked to different kbStatementId?

"variantType": "mut",
"kbVariantId": "#999:9999"
}
]
Copy link
Member

Choose a reason for hiding this comment

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

Is kbMatchedStatements only for fully-matched statements? And kbMatchedStatements.kbMatches an array where each element correspond to an element in kbMatches?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Each element in the kbmatch array will be created as a new kbmatch record in the db. It doesn't have to be defined anywhere else in the json (except the observed variant of course).

"kbVariant": "APC frameshift mutation",
"variant": "a12458ff7c8eb865656fd61f670cd0d5",
"variantType": "mut",
"kbVariantId": "#161:769",
Copy link
Member

Choose a reason for hiding this comment

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

Further down (line 391 and others), kbStatementId is assigned a long description; I suppose it's meant to be an RID now?

for (const createStatement of statementData) {
const [statement] = await db.models.kbMatchedStatements.findOrCreate({
where: {
// TODO revert this test change
Copy link
Contributor

Choose a reason for hiding this comment

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

is this still a todo for future tickets @elewis2?

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