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

Fix liftover #5326

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ About changelog [here](https://keepachangelog.com/en/1.0.0/)
- Updated color scheme for variant assessment badges that were hard to see in light mode, notably Risk Factor (#5318)
- Avoid page timeout by skipping HGVS validations in ClinVar multistep submission for non-MANE transcripts from variants in build 38 (#5302)
- Sashimi view page displaying an error message when Ensembl REST API (LiftOver) is not available (#5322)
- Refactored the liftover functionality to avoid using the old Ensembl REST API (#5326)

## [4.98]
### Added
Expand Down
28 changes: 8 additions & 20 deletions scout/utils/ensembl_rest_clients.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,18 @@
LOG = logging.getLogger(__name__)

HEADERS = {"Content-type": "application/json"}
RESTAPI_37 = "https://grch37.rest.ensembl.org"
Copy link
Member Author

Choose a reason for hiding this comment

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

Back in the day we had this distinction because we had more REST API functions. This is no longer needed now, I think

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah wait a sec, there is one functionality that is not liftover. I'll convert this to a draft again

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah no, it's just a test, nothing used in the code. I think I'll just remove it

RESTAPI_38 = "https://rest.ensembl.org"
RESTAPI_URL = "https://rest.ensembl.org"


class EnsemblRestApiClient:
"""A class handling requests and responses to and from the Ensembl REST APIs.
Endpoints for human build 37: https://grch37.rest.ensembl.org
Endpoints for human build 38: http://rest.ensembl.org/
Endpoint: http://rest.ensembl.org/
Documentation: https://github.com/Ensembl/ensembl-rest/wiki
doi:10.1093/bioinformatics/btu613
"""

def __init__(self, build="37"):
if build == "38":
self.server = RESTAPI_38
else:
self.server = RESTAPI_37
def __init__(self):
self.server = RESTAPI_URL

def build_url(self, endpoint, params=None):
"""Build an url to query ensembml"""
Expand Down Expand Up @@ -63,18 +58,11 @@ def send_request(url) -> Optional[dict]:
flash(error)
return data

def liftover(self, build, chrom, start, end=None):
def liftover(
self, build: str, chrom: str, start: int, end: Optional[int] = None
) -> Optional[dict]:
"""Perform variant liftover using Ensembl REST API

Args:
build(str): genome build: "37" or "38"
chrom(str): 1-22,X,Y,MT,M
start(int): start coordinate
stop(int): stop coordinate or None

Returns:
mappings(list of dict): example:
example: https://rest.ensembl.org/map/human/GRCh37/X:1000000..1000100:1/GRCh38?content-type=application/json
example: https://rest.ensembl.org/map/human/GRCh37/X:1000000..1000100:1/GRCh38?content-type=application/json
"""

build = "GRCh38" if "38" in str(build) else "GRCh37"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from scout.server.blueprints.alignviewers import controllers
from scout.server.extensions import config_igv_tracks, store
from scout.utils.ensembl_rest_clients import RESTAPI_URL


@responses.activate
Expand All @@ -15,7 +16,7 @@ def test_make_sashimi_tracks_variant_38(app, case_obj, ensembl_liftover_response
test_variant = store.variant_collection.find_one({"hgnc_symbols": ["POT1"]})

# GIVEN a patched response from Ensembl liftover API
url = f'https://grch37.rest.ensembl.org/map/human/GRCh37/{test_variant["chromosome"]}:{test_variant["position"]}..{test_variant["end"]}/GRCh38?content-type=application/json'
url = f'{RESTAPI_URL}/map/human/GRCh37/{test_variant["chromosome"]}:{test_variant["position"]}..{test_variant["end"]}/GRCh38?content-type=application/json'
responses.add(
responses.GET,
url,
Expand Down Expand Up @@ -56,7 +57,7 @@ def test_make_sashimi_tracks_variant_37(app, case_obj, ensembl_liftover_response
test_variant = store.variant_collection.find_one({"hgnc_symbols": ["POT1"]})

# GIVEN a patched response from Ensembl liftover API
url = f'https://grch37.rest.ensembl.org/map/human/GRCh37/{test_variant["chromosome"]}:{test_variant["position"]}..{test_variant["end"]}/GRCh38?content-type=application/json'
url = f'{RESTAPI_URL}/map/human/GRCh37/{test_variant["chromosome"]}:{test_variant["position"]}..{test_variant["end"]}/GRCh38?content-type=application/json'
responses.add(
responses.GET,
url,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import responses
from flask import session, url_for

from scout.utils.ensembl_rest_clients import RESTAPI_URL


def test_remote_static_no_auth(app):
"""Test endpoint that serves alignment files as non-logged user"""
Expand Down Expand Up @@ -152,7 +154,7 @@ def test_sashimi_igv(app, user_obj, case_obj, variant_obj, ensembl_liftover_resp
# GIVEN a mocked response from the Ensembl liftover service
chromosome = variant_obj["chromosome"]
position = variant_obj["position"]
mocked_liftover_url = f"https://grch37.rest.ensembl.org/map/human/GRCh37/{chromosome}:{position}..{position}/GRCh38?content-type=application/json"
mocked_liftover_url = f"{RESTAPI_URL}/map/human/GRCh37/{chromosome}:{position}..{position}/GRCh38?content-type=application/json"
responses.add(
responses.GET,
mocked_liftover_url,
Expand Down
4 changes: 2 additions & 2 deletions tests/server/blueprints/variant/test_variant_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

import responses
from flask import url_for
from flask_login import current_user

from scout.server.extensions import store
from scout.utils.ensembl_rest_clients import RESTAPI_URL


@responses.activate
Expand All @@ -24,7 +24,7 @@ def test_marrvel_link_38(app, case_obj):
store.variant_collection.insert_one(test_variant)

# GIVEN that the variant can be lifted over to build 37
url = f"https://grch37.rest.ensembl.org/map/human/GRCh38/{test_variant['chromosome']}:{test_variant['position']}..{test_variant['position']}/GRCh37?content-type=application/json"
url = f"{RESTAPI_URL}/map/human/GRCh38/{test_variant['chromosome']}:{test_variant['position']}..{test_variant['position']}/GRCh37?content-type=application/json"

liftover_mappings = {
"mappings": [
Expand Down
14 changes: 4 additions & 10 deletions tests/utils/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,15 @@ def fixture_ensembl_biomart_xml_query():


@pytest.fixture
def ensembl_biomart_client_37():
def ensembl_biomart_client():
"""Return a client to the ensembl biomart, build 37"""
return ensembl_rest_clients.EnsemblBiomartClient("37")


@pytest.fixture
def ensembl_rest_client_37():
"""Return a client to the ensembl rest api, build 37"""
return ensembl_rest_clients.EnsemblRestApiClient("37")


@pytest.fixture
def ensembl_rest_client_38():
"""Return a client to the ensembl rest api, build 38"""
return ensembl_rest_clients.EnsemblRestApiClient("38")
def ensembl_rest_client():
"""Return a client to the Ensembl REST API."""
return ensembl_rest_clients.EnsemblRestApiClient()


@pytest.fixture
Expand Down
38 changes: 9 additions & 29 deletions tests/utils/test_ensembl_rest_clients.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,53 +4,33 @@
from requests.exceptions import MissingSchema
from requests.models import Response

from scout.utils.ensembl_rest_clients import RESTAPI_37
from scout.utils.ensembl_rest_clients import RESTAPI_URL


@responses.activate
def test_liftover(ensembl_rest_client_37, ensembl_liftover_response):
def test_liftover(ensembl_rest_client, ensembl_liftover_response):
"""Test send request for coordinates liftover"""
# GIVEN a patched response from Ensembl
url = f"{RESTAPI_37}/map/human/GRCh37/X:1000000..1000100/GRCh38?content-type=application/json"
url = f"{RESTAPI_URL}/map/human/GRCh37/X:1000000..1000100/GRCh38?content-type=application/json"
responses.add(
responses.GET,
url,
json=ensembl_liftover_response,
status=200,
)
client = ensembl_rest_client_37
client = ensembl_rest_client
# WHEN sending the liftover request the function should return a mapped locus
mapped_coords = client.liftover("37", "X", 1000000, 1000100)
assert mapped_coords[0]["mapped"]


@responses.activate
def test_send_gene_request(ensembl_gene_response, ensembl_rest_client_37):
"""Test send request with correct params and endpoint"""
url = f"{RESTAPI_37}/overlap/id/ENSG00000103591?feature=gene"
Copy link
Member Author

Choose a reason for hiding this comment

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

This overlap functionality is not even used in Scout so the test is pointless

client = ensembl_rest_client_37
responses.add(
responses.GET,
url,
json=ensembl_gene_response,
status=200,
)
data = client.send_request(url)

# get all gene for the ensembl gene, They should be a list of items
assert data[0]["assembly_name"] == "GRCh37"
assert data[0]["external_name"] == "AAGAB"
assert data[0]["start"]
assert data[0]["end"]


def test_send_request_fakey_url(mock_app, ensembl_rest_client_37, mocker):
def test_send_request_fakey_url(mock_app, ensembl_rest_client, mocker):
"""Test the Ensembl REST client with an URL that is raising missing schema error."""

# GIVEN a completely invalid URL
url = "fakeyurl"
# GIVEN a patched Ensembl client
client = ensembl_rest_client_37
client = ensembl_rest_client
mocker.patch("requests.get", side_effect=MissingSchema("Invalid URL"))

# THEN the client should return no content
Expand All @@ -59,12 +39,12 @@ def test_send_request_fakey_url(mock_app, ensembl_rest_client_37, mocker):
assert data is None


def test_send_request_unavaailable(mock_app, ensembl_rest_client_37, mocker):
def test_send_request_unavaailable(mock_app, ensembl_rest_client, mocker):
"""Test the Ensembl REST client with an URL that is not available (500 error)."""

url = f"{RESTAPI_37}/fakeyurl"
url = f"{RESTAPI_URL}/fakeyurl"
# GIVEN a patched Ensembl client
client = ensembl_rest_client_37
client = ensembl_rest_client

# GIVEN a mocked 550 response from Ensembl
mock_response = Response()
Expand Down