Skip to content

Support mapper update for multi-target score sets#436

Merged
bencap merged 6 commits intorelease-2025.2.0from
mapper-multi-target
Jun 11, 2025
Merged

Support mapper update for multi-target score sets#436
bencap merged 6 commits intorelease-2025.2.0from
mapper-multi-target

Conversation

@sallybg
Copy link
Copy Markdown
Contributor

@sallybg sallybg commented May 22, 2025

No description provided.

@sallybg sallybg requested a review from bencap May 22, 2025 21:14
Copy link
Copy Markdown
Collaborator

@bencap bencap left a comment

Choose a reason for hiding this comment

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

This all looks good to me. The tests will need to be edited to reflect the new mapping results style, and we should also add more tests for accession based and multi target mappings now that they are supported.

Comment on lines 404 to 405
# TODO(VariantEffect/dcd-mapping2#3) after adding accession-based score set mapping support:
# this also assumes that the score set is based on a target sequence, not a target accession
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This TODO is now addressed by these changes correct?

Comment on lines +251 to +255
ANNOTATION_LAYERS = {
"g": "genomic",
"p": "protein",
"c": "cdna",
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could we move this to a new file src/mavedb/lib/mapping.py? Along with this layer dictionary, we can also move the VRSMap class from src/mavedb/data_providers/services.py to that file. That would better centralize these objects for use across the code base.

pre_mapped_metadata[ANNOTATION_LAYERS[annotation_layer]] = {
k: layer_premapped[k]
for k in set(list(layer_premapped.keys()))
- excluded_pre_mapped_keys # TODO does this work if no 'sequence' key?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes :)

sallybg added 2 commits June 4, 2025 11:54
variant_mapper_manager previously failed because of an incorrectly placed
'where' function and because we did two related db queries, one for a score set
and one for target gene(s) within that score set, which resulted in an error
when adding the score set changes to the db. Instead, query to select the score set,
and then loop through the score set's target genes rather than querying the target genes
table in the db.
@bencap bencap merged commit 0e52962 into release-2025.2.0 Jun 11, 2025
8 checks passed
@bencap bencap deleted the mapper-multi-target branch June 11, 2025 18:45
@bencap bencap mentioned this pull request Jun 11, 2025
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.

VRS mapping for accession-based targets Mapping Score Sets with more than 1 Target

2 participants