diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 7804a8a0..32a96b2f 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -32,6 +32,18 @@ jobs: run: | pip install ruff ruff format --check pori_python tests + - name: short tests + run: | + pip list + pytest --junitxml=junit/test-results-${{ matrix.python-version }}.xml --cov pori_python --cov-report term --cov-report xml + env: + IPR_USER: ${{ secrets.IPR_TEST_USER }} + IPR_PASS: ${{ secrets.IPR_TEST_PASSWORD }} + GRAPHKB_USER: ${{ secrets.GKB_TEST_USER }} + GRAPHKB_PASS: ${{ secrets.GKB_TEST_PASS }} + GRAPHKB_URL: ${{ secrets.GKB_TEST_URL }} + EXCLUDE_INTEGRATION_TESTS: 1 + if: matrix.python-version != 3.11 - name: Full Tests with pytest run: | pip list @@ -39,19 +51,19 @@ jobs: env: IPR_USER: ${{ secrets.IPR_TEST_USER }} IPR_PASS: ${{ secrets.IPR_TEST_PASSWORD }} + IPR_URL: ${{ secrets.IPR_TEST_URL }} + IPR_INTEGRATION_TEST_URL: ${{ secrets.IPR_INTEGRATION_TEST_URL }} GRAPHKB_USER: ${{ secrets.GKB_TEST_USER }} GRAPHKB_PASS: ${{ secrets.GKB_TEST_PASS }} GRAPHKB_URL: ${{ secrets.GKB_TEST_URL }} - # SDEV-3381 - Turn off integration tests temporarily, till efficiency is increased - # turn on integration tests for one python version only - EXCLUDE_INTEGRATION_TESTS: ${{ matrix.python-version != '3.11' }} + if: matrix.python-version == 3.11 - name: Upload pytest test results uses: actions/upload-artifact@master with: name: pytest-results-${{ matrix.python-version }} path: junit/test-results-${{ matrix.python-version }}.xml # Use always() to always run this step to publish test results when there are test failures - if: matrix.python-version == 3.9 + if: matrix.python-version == 3.11 - name: Update code coverage report to CodeCov uses: codecov/codecov-action@v3 with: @@ -61,4 +73,4 @@ jobs: env_vars: OS,PYTHON name: codecov-umbrella fail_ci_if_error: true - if: matrix.python-version == 3.9 + if: matrix.python-version == 3.11 diff --git a/.github/workflows/quick-pytest.yml b/.github/workflows/quick-pytest.yml index aef2b56d..5bdf6534 100644 --- a/.github/workflows/quick-pytest.yml +++ b/.github/workflows/quick-pytest.yml @@ -35,6 +35,7 @@ jobs: IPR_USER: ${{ secrets.IPR_TEST_USER }} IPR_PASS: ${{ secrets.IPR_TEST_PASSWORD }} IPR_URL: ${{ secrets.IPR_TEST_URL }} + IPR_INTEGRATION_TEST_URL: ${{ secrets.IPR_INTEGRATION_TEST_URL }} GRAPHKB_USER: ${{ secrets.GKB_TEST_USER }} GRAPHKB_PASS: ${{ secrets.GKB_TEST_PASS }} GRAPHKB_URL: ${{ secrets.GKB_TEST_URL }} diff --git a/.gitignore b/.gitignore index f634e823..f001284b 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ htmlcov # common virtual environment names venv* env +.venv # editors .idea diff --git a/pori_python/graphkb/match.py b/pori_python/graphkb/match.py index 29c8cf32..97a803ee 100644 --- a/pori_python/graphkb/match.py +++ b/pori_python/graphkb/match.py @@ -2,7 +2,7 @@ Functions which return Variants from GraphKB which match some input variant definition """ -from typing import Dict, List, Optional, Set, Union, cast +from typing import Dict, List, Optional, Union, cast from pori_python.types import ( BasicPosition, @@ -31,14 +31,7 @@ looks_like_rid, stringifyVariant, ) -from .vocab import ( - get_equivalent_terms, - get_term_by_name, - get_term_tree, - get_terms_set, -) - -FEATURES_CACHE: Set[str] = set() +from .vocab import get_equivalent_terms, get_term_by_name, get_term_tree, get_terms_set def get_equivalent_features( @@ -97,8 +90,6 @@ def get_equivalent_features( filters.append( {'OR': [{'sourceIdVersion': source_id_version}, {'sourceIdVersion': None}]} ) - elif FEATURES_CACHE and gene_name.lower() not in FEATURES_CACHE and not ignore_cache: - return [] else: filters.append({'OR': [{'sourceId': gene_name}, {'name': gene_name}]}) @@ -111,22 +102,6 @@ def get_equivalent_features( ) -def cache_missing_features(conn: GraphKBConnection) -> None: - """ - Create a cache of features that exist to avoid repeatedly querying - for missing features - """ - genes = cast( - List[Ontology], - conn.query({'target': 'Feature', 'returnProperties': ['name', 'sourceId'], 'neighbors': 0}), - ) - for gene in genes: - if gene['name']: - FEATURES_CACHE.add(gene['name'].lower()) - if gene['sourceId']: - FEATURES_CACHE.add(gene['sourceId'].lower()) - - def match_category_variant( conn: GraphKBConnection, reference_name: str, diff --git a/pori_python/graphkb/util.py b/pori_python/graphkb/util.py index a2a9bb14..b92c85b2 100644 --- a/pori_python/graphkb/util.py +++ b/pori_python/graphkb/util.py @@ -1,27 +1,27 @@ -import requests -from requests.adapters import HTTPAdapter - -import hashlib import json import logging import os import re import time from datetime import datetime -from typing import Any, Dict, Iterable, List, Optional, Union, cast -from urllib3.util.retry import Retry +from typing import Dict, Iterable, List, Optional, Union, cast from urllib.parse import urlsplit +import requests +from requests.adapters import HTTPAdapter +from requests_cache import CachedSession +from requests_ratelimiter import LimiterAdapter +from urllib3.util.retry import Retry + from pori_python.types import ParsedVariant, PositionalVariant, Record from .constants import DEFAULT_LIMIT, TYPES_TO_NOTATION, AA_3to1_MAPPING -QUERY_CACHE: Dict[Any, Any] = {} - # name the logger after the package to make it simple to disable for packages using this one as a dependency # https://stackoverflow.com/questions/11029717/how-do-i-disable-log-messages-from-the-requests-library logger = logging.getLogger('graphkb') +DEFAULT_LIMITER = LimiterAdapter(per_second=3) def convert_to_rid_list(records: Iterable[Record]) -> List[str]: @@ -88,11 +88,17 @@ def millis_interval(start: datetime, end: datetime) -> int: return millis -def cache_key(request_body) -> str: - """Create a cache key for a query request to GraphKB.""" - body = json.dumps(request_body, sort_keys=True) - hash_code = hashlib.md5(f'/query{body}'.encode('utf-8')).hexdigest() - return hash_code +def cache_filter(response): + """ + Only cache POST requests when they are for the /query endpoint. Otherwise they are creating content + and should not be cached + """ + request = response.request + + if request.method == 'POST': + return request.url.rstrip('/').endswith('/query') + + return True class GraphKBConnection: @@ -102,23 +108,89 @@ def __init__( username: str = '', password: str = '', use_global_cache: bool = True, + cache_name: str = '', + only_if_cached: bool = False, + session: Optional[requests.Session] = None, + limiter_kwargs: Optional[dict] = None, + **session_kwargs, ): - self.http = requests.Session() - retries = Retry( + """ + Docstring for __init__ + + Args: + - use_global_cache: cache requests across all requests to GKB + - cache_name: Path or connection URL to the database which stors the requests cache. see https://requests-cache.readthedocs.io/en/v0.6.4/user_guide.html#cache-name + - only_if_cached: this will set the cache-control header for all requests to only-if-cached which will raise 504 errors if a request does not exist in the cache already rather than making a new network request + """ + session_cls = requests.Session + if session is not None: + if limiter_kwargs is not None: + raise NotImplementedError('cannot add limiter to an existing session') + if use_global_cache: + raise NotImplementedError( + 'the use_global_cache parameter should not be used with a custom session' + ) + if cache_name: + raise NotImplementedError( + 'cache_name should not be used with a custom input session' + ) + if not use_global_cache and cache_name: + raise NotImplementedError('cache_name only applies when use_global_cache is True') + if only_if_cached and not use_global_cache: + raise NotImplementedError( + 'only_if_cached=True and use_global_cache=False are conflicting settings' + ) + + if use_global_cache: + session_cls = CachedSession + if not cache_name: + session_kwargs['backend'] = 'memory' + else: + session_kwargs['cache_name'] = cache_name + session_kwargs['allowable_methods'] = ['GET', 'POST'] + session_kwargs['filter_fn'] = cache_filter + session_kwargs['cache_control'] = True + + if 'PYTEST_CURRENT_TEST' in os.environ or only_if_cached: + if limiter_kwargs is not None: + logging.warning( + 'rate limiting is by default turned off for tests and cache-only queries. Setting limiter to None' + ) + limiter_kwargs = None + + if not session: + self.http = session_cls(**session_kwargs) + else: + self.http = session + + max_retries = Retry( total=100, connect=5, status=5, backoff_factor=5, status_forcelist=[429, 500, 502, 503, 504], ) - self.http.mount('https://', HTTPAdapter(max_retries=retries)) + + if limiter_kwargs: + adapter = LimiterAdapter(max_retries=max_retries, **limiter_kwargs) + else: + adapter = HTTPAdapter(max_retries=max_retries) + + if not only_if_cached: + # do not need adapters at all for cache-only hits + self.http.mount('http://', adapter) + self.http.mount('https://', adapter) + self.only_if_cached = only_if_cached + self.token = '' self.token_kc = '' self.url = url self.username = username self.password = password - self.headers = {'Accept': 'application/json', 'Content-Type': 'application/json'} - self.cache: Dict[Any, Any] = {} if not use_global_cache else QUERY_CACHE + self.headers = { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + } self.request_count = 0 self.first_request: Optional[datetime] = None self.last_request: Optional[datetime] = None @@ -137,7 +209,16 @@ def load(self) -> Optional[float]: return self.request_count * 1000 / msec return None - def request(self, endpoint: str, method: str = 'GET', **kwargs) -> Dict: + def request( + self, + endpoint: str, + method: str = 'GET', + headers: Optional[dict[str, str]] = None, + ignore_cache=False, + force_refresh=False, + only_if_cached=False, + **kwargs, + ) -> Dict: """Request wrapper to handle adding common headers and logging. Args: @@ -147,6 +228,13 @@ def request(self, endpoint: str, method: str = 'GET', **kwargs) -> Dict: Returns: dict: the json response as a python dict """ + if headers is None: + headers = {} + + if ignore_cache or force_refresh: + headers['Cache-Control'] = 'no-cache' + elif only_if_cached or self.only_if_cached: + headers['Cache-Control'] = 'only-if-cached' url = join_url(self.url, endpoint) self.request_count += 1 connect_timeout = 7 @@ -158,6 +246,11 @@ def request(self, endpoint: str, method: str = 'GET', **kwargs) -> Dict: if endpoint in ['query', 'parse']: timeout = (connect_timeout, read_timeout) + request_headers = {} + request_headers.update(self.headers) + if headers is not None: + request_headers.update(headers) + start_time = datetime.now() if not self.first_request: @@ -176,11 +269,12 @@ def request(self, endpoint: str, method: str = 'GET', **kwargs) -> Dict: try: if need_refresh_login: self.refresh_login() + request_headers.update(self.headers) need_refresh_login = False self.request_count += 1 - resp = requests.request( - method, url, headers=self.headers, timeout=timeout, **kwargs + resp = self.http.request( + method, url, headers=request_headers, timeout=timeout, **kwargs ) if resp.status_code == 401 or resp.status_code == 403: logger.debug(f'/{endpoint} - {resp.status_code} - retrying') @@ -293,39 +387,29 @@ def login(self, username: str, password: str, pori_demo: bool = False) -> None: def refresh_login(self) -> None: self.login(self.username, self.password) - def set_cache_data(self, request_body: Dict, result: List[Record]) -> None: - """Explicitly add a query to the cache.""" - hash_code = cache_key(request_body) - self.cache[hash_code] = result - def query( self, request_body: Dict = {}, paginate: bool = True, - ignore_cache: bool = False, - force_refresh: bool = False, limit: int = DEFAULT_LIMIT, + **kwargs, ) -> List[Record]: """ Query GraphKB """ - result: List[Record] = [] - hash_code = '' - - if not ignore_cache and paginate: - hash_code = cache_key(request_body) - if hash_code in self.cache and not force_refresh: - return self.cache[hash_code] + result: List[Record] = [] while True: - content = self.post('query', data={**request_body, 'limit': limit, 'skip': len(result)}) + content = self.post( + 'query', + data={**request_body, 'limit': limit, 'skip': len(result)}, + **kwargs, + ) records = content['result'] result.extend(records) if len(records) < limit or not paginate: break - if not ignore_cache and paginate: - self.cache[hash_code] = result return result def parse(self, hgvs_string: str, requireFeatures: bool = False) -> ParsedVariant: @@ -500,7 +584,9 @@ def stripDisplayName(displayName: str, withRef: bool = True, withRefSeq: bool = def stringifyVariant( - variant: Union[PositionalVariant, ParsedVariant], withRef: bool = True, withRefSeq: bool = True + variant: Union[PositionalVariant, ParsedVariant], + withRef: bool = True, + withRefSeq: bool = True, ) -> str: """ Convert variant record to a string representation (displayName/hgvs) diff --git a/pori_python/graphkb/vocab.py b/pori_python/graphkb/vocab.py index bb96e5f5..e1a099c3 100644 --- a/pori_python/graphkb/vocab.py +++ b/pori_python/graphkb/vocab.py @@ -1,10 +1,12 @@ -from typing import Callable, Dict, Iterable, List, Set, cast, Union +from typing import Callable, Dict, Iterable, List, Set, Union, cast from pori_python.types import Ontology from . import GraphKBConnection from .util import convert_to_rid_list +DEFAULT_RETURN_PROPERTIES = ['sourceId', 'sourceIdVersion', 'deprecated', 'name', '@rid'] + def query_by_name(ontology_class: str, base_term_name: Union[str, list[str]]) -> Dict: return {'target': ontology_class, 'filters': {'name': base_term_name}} @@ -17,6 +19,7 @@ def get_equivalent_terms( ontology_class: str = 'Vocabulary', ignore_cache: bool = False, build_base_query: Callable = query_by_name, + return_properties=DEFAULT_RETURN_PROPERTIES, ) -> List[Ontology]: """ Get a list of terms equivalent to the current term up to the root term @@ -35,7 +38,7 @@ def get_equivalent_terms( 'target': {'target': base_records, 'queryType': 'descendants'}, 'queryType': 'similarTo', 'treeEdges': [], - 'returnProperties': ['sourceId', 'sourceIdVersion', 'deprecated', 'name', '@rid'], + 'returnProperties': return_properties, }, ignore_cache=ignore_cache, ), @@ -54,13 +57,7 @@ def get_equivalent_terms( 'target': {'target': root_records, 'queryType': 'descendants'}, 'queryType': 'similarTo', 'treeEdges': [], - 'returnProperties': [ - 'sourceId', - 'sourceIdVersion', - 'deprecated', - 'name', - '@rid', - ], + 'returnProperties': ['@rid'], }, ignore_cache=ignore_cache, ) @@ -78,6 +75,7 @@ def get_term_tree( include_superclasses: bool = True, ignore_cache: bool = False, build_base_query: Callable = query_by_name, + return_properties=DEFAULT_RETURN_PROPERTIES, ) -> List[Ontology]: """ Get terms equivalent to the base term by traversing the subclassOf tree and expanding related @@ -105,7 +103,7 @@ def get_term_tree( 'target': {'target': base_records, 'queryType': 'ancestors'}, 'queryType': 'similarTo', 'treeEdges': [], - 'returnProperties': ['sourceId', 'sourceIdVersion', 'deprecated', 'name', '@rid'], + 'returnProperties': return_properties, }, ignore_cache=ignore_cache, ), @@ -119,6 +117,7 @@ def get_term_tree( ontology_class=ontology_class, ignore_cache=ignore_cache, build_base_query=build_base_query, + return_properties=return_properties, ) else: parent_terms = [] @@ -181,9 +180,6 @@ def get_terms_set( ) -> Set[str]: """Get a set of vocabulary rids given some base/parent term names.""" base_terms = [base_terms] if isinstance(base_terms, str) else base_terms - cache_key = tuple(sorted(base_terms)) - if graphkb_conn.cache.get(cache_key, None) and not ignore_cache: - return graphkb_conn.cache[cache_key] terms = set() for base_term in base_terms: terms.update( @@ -193,6 +189,4 @@ def get_terms_set( ) ) ) - if not ignore_cache: - graphkb_conn.cache[cache_key] = terms return terms diff --git a/pori_python/ipr/content.spec.json b/pori_python/ipr/content.spec.json index c994ba6f..f35b15bc 100644 --- a/pori_python/ipr/content.spec.json +++ b/pori_python/ipr/content.spec.json @@ -564,6 +564,10 @@ "hrd": { "properties": { "kbCategory": { + "examples": [ + "homologous recombination deficiency strong signature", + "homologous recombination deficiency moderate signature" + ], "type": "string" }, "score": { diff --git a/pori_python/ipr/ipr.py b/pori_python/ipr/ipr.py index 9a9afb38..26c0a2dc 100644 --- a/pori_python/ipr/ipr.py +++ b/pori_python/ipr/ipr.py @@ -322,15 +322,26 @@ def create_key_alterations( counts[type_mapping[variant_type]].add(variant_key) - if variant_type == 'exp': - alterations.append(f'{variant.get("gene", "")} ({variant.get("expressionState")})') - elif variant_type == 'cnv': - alterations.append(f'{variant.get("gene", "")} ({variant.get("cnvState")})') - # only show germline if relevant - elif kb_match['category'] in GERMLINE_BASE_TERMS and variant.get('germline'): - alterations.append(f'germline {variant["variant"]}') + # DEVSU-2013 returning an alteration object for IPR API to look up an observed variant for association at report creation. + # variantType: the type of variant (mut, cnv, sv, exp) + # variant: the observed variant's key property (eg: e90293983749cf4) + # Example value: {'variantType': 'mut', 'variant': 'e90293983749cf4'} + if kb_match['category'] in GERMLINE_BASE_TERMS: + # only include germline-category matches when the observed variant is germline + if variant.get('germline'): + alterations.append( + { + 'variantType': variant_type, + 'variant': variant['key'], + }, + ) else: - alterations.append(variant['variant']) + alterations.append( + { + 'variantType': variant_type, + 'variant': variant['key'], + }, + ) counted_variants = set.union(*counts.values()) counts['variantsUnknown'] = set() @@ -340,8 +351,17 @@ def create_key_alterations( if variant['variant'] and variant['key'] not in counted_variants: counts['variantsUnknown'].add(variant['key']) + unique_alterations = [] + seen = set() + for alt in alterations: + dedupe_key = (alt['variantType'], alt['variant']) + if dedupe_key in seen: + continue + seen.add(dedupe_key) + unique_alterations.append(alt) + return ( - [{'geneVariant': alt} for alt in set(alterations)], + unique_alterations, {k: len(v) for k, v in counts.items()}, ) diff --git a/pori_python/ipr/main.py b/pori_python/ipr/main.py index d261cbd5..f4cafffa 100644 --- a/pori_python/ipr/main.py +++ b/pori_python/ipr/main.py @@ -3,12 +3,13 @@ import argparse import datetime import json -import jsonschema.exceptions import logging import os -import pandas as pd from argparse import ArgumentDefaultsHelpFormatter, ArgumentParser -from typing import Callable, Dict, List, Optional, Sequence, Set +from collections.abc import Callable, Sequence + +import jsonschema.exceptions +import pandas as pd from pori_python.graphkb import GraphKBConnection from pori_python.graphkb.genes import get_gene_information @@ -33,8 +34,8 @@ preprocess_cosmic, preprocess_expression_variants, preprocess_hla, - preprocess_msi, preprocess_hrd, + preprocess_msi, preprocess_signature_variants, preprocess_small_mutations, preprocess_structural_variants, @@ -42,14 +43,14 @@ validate_report_content, ) from .ipr import ( + add_transcript_flags, create_key_alterations, filter_structural_variants, germline_kb_matches, get_kb_disease_matches, get_kb_matches_sections, - select_expression_plots, get_variant_flags, - add_transcript_flags, + select_expression_plots, ) from .summary import auto_analyst_comments, get_ipr_analyst_comments from .therapeutic_options import create_therapeutic_options @@ -65,7 +66,7 @@ def file_path(path: str) -> str: if not os.path.exists(path): - raise argparse.ArgumentTypeError(f'{repr(path)} is not a valid filename. does not exist') + raise argparse.ArgumentTypeError(f'{path!r} is not a valid filename. does not exist') return path @@ -215,7 +216,7 @@ def command_interface() -> None: ) -def clean_unsupported_content(upload_content: Dict, ipr_spec: Dict = {}) -> Dict: +def clean_unsupported_content(upload_content: dict, ipr_spec: dict = {}) -> dict: """Remove unsupported content. This content is either added to facilitate creation or to support upcoming and soon to be supported content that we would like @@ -223,7 +224,7 @@ def clean_unsupported_content(upload_content: Dict, ipr_spec: Dict = {}) -> Dict """ if ( ipr_spec - and 'components' in ipr_spec.keys() + and 'components' in ipr_spec and 'schemas' in ipr_spec['components'].keys() and 'genesCreate' in ipr_spec['components']['schemas'].keys() and 'properties' in ipr_spec['components']['schemas']['genesCreate'].keys() @@ -253,7 +254,7 @@ def clean_unsupported_content(upload_content: Dict, ipr_spec: Dict = {}) -> Dict ) # remove any unhandled incompatible keys - removed_keys: Dict[str, int] = {} + removed_keys: dict[str, int] = {} for gene in upload_content['genes']: unsupported_keys = [key for key in gene.keys() if key not in genes_spec] for key in unsupported_keys: @@ -315,7 +316,7 @@ def clean_unsupported_content(upload_content: Dict, ipr_spec: Dict = {}) -> Dict return upload_content -def create_report(**kwargs) -> Dict: +def create_report(**kwargs) -> dict: logger.warning("Deprecated function 'create_report' called - use ipr_report instead") return ipr_report(**kwargs) @@ -323,7 +324,7 @@ def create_report(**kwargs) -> Dict: def ipr_report( username: str, password: str, - content: Dict, + content: dict, ipr_url: str = '', log_level: str = 'info', output_json_path: str = '', @@ -336,7 +337,7 @@ def ipr_report( generate_therapeutics: bool = False, generate_comments: bool = True, match_germline: bool = False, - custom_kb_match_filter: Optional[Callable] = None, + custom_kb_match_filter: Callable | None = None, async_upload: bool = False, mins_to_wait: int = 10, include_ipr_variant_text: bool = True, @@ -349,7 +350,7 @@ def ipr_report( ignore_extra_fields: bool = False, tmb_high: float = TMB_SIGNATURE_HIGH_THRESHOLD, transcript_flags: str = '', -) -> Dict: +) -> dict: """Run the matching and create the report JSON for upload to IPR. Args: @@ -431,7 +432,7 @@ def ipr_report( transcript_flags_df = load_transcript_flags(transcript_flags) # INPUT VARIANTS VALIDATION & PREPROCESSING (OBSERVED BIOMARKERS) - signature_variants: List[IprSignatureVariant] = preprocess_signature_variants( + signature_variants: list[IprSignatureVariant] = preprocess_signature_variants( [ *preprocess_cosmic(content.get('cosmicSignatures', [])), # includes dMMR *preprocess_hla(content.get('hlaTypes', [])), @@ -444,14 +445,14 @@ def ipr_report( *preprocess_hrd(content.get('hrd', None)), ] ) - small_mutations: List[IprSmallMutationVariant] = preprocess_small_mutations( + small_mutations: list[IprSmallMutationVariant] = preprocess_small_mutations( content.get('smallMutations', []) ) - structural_variants: List[IprFusionVariant] = preprocess_structural_variants( + structural_variants: list[IprFusionVariant] = preprocess_structural_variants( content.get('structuralVariants', []) ) - copy_variants: List[IprCopyVariant] = preprocess_copy_variants(content.get('copyVariants', [])) - expression_variants: List[IprExprVariant] = preprocess_expression_variants( + copy_variants: list[IprCopyVariant] = preprocess_copy_variants(content.get('copyVariants', [])) + expression_variants: list[IprExprVariant] = preprocess_expression_variants( content.get('expressionVariants', []) ) @@ -459,7 +460,7 @@ def ipr_report( if expression_variants: check_comparators(content, expression_variants) - genes_with_variants: Set[str] = check_variant_links( + genes_with_variants: set[str] = check_variant_links( small_mutations, expression_variants, copy_variants, structural_variants ) @@ -478,12 +479,12 @@ def ipr_report( # Matching disease RIDs from GraphKB using term tree # (Will raise uncatched error if no match) - disease_match_records: list[Dict] = get_kb_disease_matches(graphkb_conn, kb_disease_match) + disease_match_records: list[dict] = get_kb_disease_matches(graphkb_conn, kb_disease_match) disease_match_rids: list[str] = [item['@rid'] for item in disease_match_records] disease_match_names: list[str] = [item['name'] for item in disease_match_records] # GKB MATCHING (AKA ANNOTATION) - gkb_matches: List[Hashabledict] = annotate_variants( + gkb_matches: list[Hashabledict] = annotate_variants( graphkb_conn=graphkb_conn, interactive=interactive, disease_matches=disease_match_rids, @@ -654,7 +655,16 @@ def ipr_report( output, mins_to_wait, async_upload, ignore_extra_fields ) logger.info(ipr_result) - output.update(ipr_result) + if async_upload and 'report' in ipr_result: + # async uploads return the report nested under 'report' alongside + # job-tracking fields (eg. 'ident' of the job, 'jobStatus') + # full contents of 'report' aren't needed and can be ignored. + # no 'message' value is available for async results + # but 'ident' is, and is required by users. + output['ident'] = ipr_result['report']['ident'] + else: + # for sync uploads, add 'ident' and 'message' fields to the output. + output.update(ipr_result) except Exception as err: upload_error = err logger.error(f'ipr_conn.upload_report failed: {err}', exc_info=True) diff --git a/pori_python/ipr/summary.py b/pori_python/ipr/summary.py index cfe60868..05025c2d 100644 --- a/pori_python/ipr/summary.py +++ b/pori_python/ipr/summary.py @@ -372,11 +372,11 @@ def prep_single_ipr_variant_comment(variant_text): Returns: section: html-formatted string """ - cancer_type = ','.join(variant_text['cancerType']) + cancer_type = ', '.join(variant_text['cancerType']) if not cancer_type: cancer_type = 'no specific cancer types' cancer_type = f' ({cancer_type})' - section = [f'

{variant_text["variantName"]}{cancer_type}

'] + section = [f'

{variant_text["variantName"]}{cancer_type}

'] section.append(f'

{variant_text["text"]}

') return section @@ -385,7 +385,7 @@ def get_ipr_analyst_comments( ipr_conn: IprConnection, matches: Sequence[KbMatch] | Sequence[Hashabledict], disease_name: str, - disease_match_names: [str], + disease_match_names: list[str], project_name: str, report_type: str, include_nonspecific_disease: bool = False, @@ -434,12 +434,13 @@ def get_ipr_analyst_comments( project_matches = [ item for item in itemlist - if 'project' in item.keys() and item['project']['name'] == project_name + if 'projects' in item.keys() + and project_name in [p['name'] for p in item['projects']] ] if project_matches: itemlist = project_matches elif include_nonspecific_project: - itemlist = [item for item in itemlist if 'project' not in item.keys()] + itemlist = [item for item in itemlist if 'projects' not in item.keys()] else: itemlist = [] @@ -463,7 +464,6 @@ def get_ipr_analyst_comments( ) > 0 ] - if disease_matches: itemlist = disease_matches elif include_nonspecific_disease: diff --git a/setup.cfg b/setup.cfg index 0cdeb341..8651f8a5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -18,7 +18,7 @@ known_standard_library = requests [metadata] name = pori_python -version = 1.5.0 +version = 1.6.0 url = https://github.com/bcgsc/pori_python author_email = dat@bcgsc.ca maintainer_email = dat@bcgsc.ca @@ -36,6 +36,9 @@ install_requires = requests tqdm typing_extensions>=3.7.4.2,<5 + requests-cache>=1.3.0,<2.0 + requests-ratelimiter + pyrate-limiter [options.extras_require] deploy = twine; wheel; m2r @@ -62,3 +65,8 @@ pori_python = py.typed [options.entry_points] console_scripts = ipr = pori_python.ipr.main:command_interface + + +[tool:pytest] +log_cli = true +log_cli_level = INFO diff --git a/tests/test_graphkb/test_match.py b/tests/test_graphkb/test_match.py index 7a34b900..1e6b54d1 100644 --- a/tests/test_graphkb/test_match.py +++ b/tests/test_graphkb/test_match.py @@ -563,22 +563,6 @@ def test_structural_variants(self, conn): assert type not in expected.get('does_not_matches', {}).get('type', []) -class TestCacheMissingFeatures: - def test_filling_cache(self): - mock_conn = MagicMock( - query=MagicMock( - return_value=[ - {'name': 'bob', 'sourceId': 'alice'}, - {'name': 'KRAS', 'sourceId': '1234'}, - ] - ) - ) - match.cache_missing_features(mock_conn) - assert 'kras' in match.FEATURES_CACHE - assert 'alice' in match.FEATURES_CACHE - match.FEATURES_CACHE = None - - class TestTypeScreening: # Types as class variables default_type = DEFAULT_NON_STRUCTURAL_VARIANT_TYPE diff --git a/tests/test_graphkb/test_statement.py b/tests/test_graphkb/test_statement.py index 89faafdd..550191ed 100644 --- a/tests/test_graphkb/test_statement.py +++ b/tests/test_graphkb/test_statement.py @@ -17,11 +17,21 @@ def conn() -> GraphKBConnection: @pytest.fixture() def graphkb_conn(): + """ + Mocks the query functionality required by the calls made for the categorize_relevance function + + categorize_relevance calls query twice for each term in the values of the category_base_terms object + + - get_terms_set ([term, term, term...]) + - get_term_tree(term) + - query(query(term)) + """ + def make_rid_list(*values): return [{'@rid': v} for v in values] def term_tree_calls(*final_values): - # this function makes 2 calls to conn.query here + # this function makes 2 calls to conn.query here b/c the get_terms_set function will always call query twice sets = [['fake'], final_values] return [make_rid_list(*s) for s in sets] @@ -41,7 +51,7 @@ def term_tree_calls(*final_values): query_mock = Mock() query_mock.side_effect = return_values - return Mock(query=query_mock, cache={}) + return Mock(query=query_mock) class TestCategorizeRelevance: @@ -77,12 +87,13 @@ def test_no_match(self, graphkb_conn): category = statement.categorize_relevance(graphkb_conn, 'x') assert category == '' - def test_custom_categories(self, graphkb_conn): + def test_custom_categories_not_found(self, graphkb_conn): category = statement.categorize_relevance( graphkb_conn, 'x', [('blargh', ['some', 'blargh'])] ) assert category == '' + def test_custom_categories_match(self, graphkb_conn): category = statement.categorize_relevance( graphkb_conn, '1', [('blargh', ['some', 'blargh'])] ) diff --git a/tests/test_graphkb/test_util.py b/tests/test_graphkb/test_util.py index dbbb2c2b..f7637bfe 100644 --- a/tests/test_graphkb/test_util.py +++ b/tests/test_graphkb/test_util.py @@ -1,12 +1,42 @@ +import io +import json import os -import pytest import re +import pytest +import requests +import urllib3 +from requests.adapters import HTTPAdapter +from requests_ratelimiter import LimiterAdapter + from pori_python.graphkb import GraphKBConnection, util EXCLUDE_BCGSC_TESTS = os.environ.get('EXCLUDE_BCGSC_TESTS') == '1' +class CountingAdapter(HTTPAdapter): + """Test transport adapter that returns a canned JSON response without any + real network I/O, and counts how many times it was actually invoked (i.e. + how many requests were NOT served from cache). + """ + + def __init__(self, *args, **kwargs): + self.calls = 0 + super().__init__(*args, **kwargs) + + def send(self, request, **kwargs): + self.calls += 1 + body = json.dumps({'call': self.calls}).encode('utf-8') + raw = urllib3.HTTPResponse( + body=io.BytesIO(body), + status=200, + preload_content=False, + headers={}, + request_url=request.url, + ) + return self.build_response(request, raw) + + class OntologyTerm: def __init__(self, name, sourceId, displayName): self.name = name @@ -16,7 +46,7 @@ def __init__(self, name, sourceId, displayName): @pytest.fixture(scope='module') def conn() -> GraphKBConnection: - conn = GraphKBConnection() + conn = GraphKBConnection(url=os.environ['GRAPHKB_URL']) conn.login(os.environ['GRAPHKB_USER'], os.environ['GRAPHKB_PASS']) return conn @@ -191,3 +221,173 @@ def test_get_related_terms(self, conn): subgraphType='parents', ) assert 'cancer' in disease_terms + + +class TestRateLimitingOptIn: + """Rate limiting is opt-in per-connection via the `limiter_kwargs` argument. + + Passing a truthy `limiter_kwargs` dict (e.g. `{'per_second': 3}`) mounts a + `LimiterAdapter`; leaving it unset (None/default) or empty leaves the plain + `HTTPAdapter` in place, i.e. rate limiting disabled. Regardless of + `limiter_kwargs`, rate limiting is always forced off while running under + pytest (`PYTEST_CURRENT_TEST` set) or when `only_if_cached=True`. + """ + + def test_default_has_no_rate_limiting(self): + conn = GraphKBConnection(url='http://localhost:8080') + assert isinstance(conn.http.adapters['https://'], HTTPAdapter) + assert not isinstance(conn.http.adapters['https://'], LimiterAdapter) + + def test_empty_limiter_kwargs_has_no_rate_limiting(self): + conn = GraphKBConnection(url='http://localhost:8080', limiter_kwargs={}) + assert not isinstance(conn.http.adapters['https://'], LimiterAdapter) + + def test_limiter_kwargs_forced_off_while_under_pytest(self, caplog): + # PYTEST_CURRENT_TEST is set while running under pytest, so rate + # limiting should stay disabled even when limiter_kwargs is provided. + assert 'PYTEST_CURRENT_TEST' in os.environ + conn = GraphKBConnection(url='http://localhost:8080', limiter_kwargs={'per_second': 3}) + assert not isinstance(conn.http.adapters['https://'], LimiterAdapter) + assert 'rate limiting is by default turned off' in caplog.text + + def test_limiter_kwargs_enables_rate_limiting_outside_pytest(self, monkeypatch): + monkeypatch.delenv('PYTEST_CURRENT_TEST', raising=False) + conn = GraphKBConnection(url='http://localhost:8080', limiter_kwargs={'per_second': 3}) + assert isinstance(conn.http.adapters['https://'], LimiterAdapter) + + def test_only_if_cached_forces_rate_limiting_off(self, monkeypatch, caplog): + monkeypatch.delenv('PYTEST_CURRENT_TEST', raising=False) + conn = GraphKBConnection( + url='http://localhost:8080', only_if_cached=True, limiter_kwargs={'per_second': 3} + ) + assert conn.only_if_cached is True + assert 'rate limiting is by default turned off' in caplog.text + + def test_limiter_kwargs_with_custom_session_raises(self): + with pytest.raises(NotImplementedError): + GraphKBConnection( + url='http://localhost:8080', + session=requests.Session(), + limiter_kwargs={'per_second': 3}, + ) + + +class TestCacheFilter: + """Unit tests for util.cache_filter, which decides which responses requests-cache + is allowed to store: all GETs, but only POSTs to the /query endpoint (other POSTs + create content and must never be cached). + """ + + class _FakeRequest: + def __init__(self, method, url): + self.method = method + self.url = url + + class _FakeResponse: + def __init__(self, method, url): + self.request = TestCacheFilter._FakeRequest(method, url) + + def test_get_requests_are_always_cacheable(self): + response = self._FakeResponse('GET', 'http://fake/api/statement') + assert util.cache_filter(response) is True + + @pytest.mark.parametrize('url', ['http://fake/api/query', 'http://fake/api/query/']) + def test_post_to_query_endpoint_is_cacheable(self, url): + response = self._FakeResponse('POST', url) + assert util.cache_filter(response) is True + + @pytest.mark.parametrize('url', ['http://fake/api/statement', 'http://fake/api/query/similar']) + def test_post_to_other_endpoints_is_not_cacheable(self, url): + response = self._FakeResponse('POST', url) + assert util.cache_filter(response) is False + + +class TestCachingBehavior: + """End-to-end tests that mount a counting fake adapter under GraphKBConnection's + session so we can assert whether a real request was made (adapter.calls + incremented) or the response was served from the requests-cache layer instead. + """ + + def test_repeated_get_is_served_from_cache(self): + conn = GraphKBConnection(url='http://fake-graphkb', use_global_cache=True) + adapter = CountingAdapter() + conn.http.mount('http://', adapter) + + first = conn.http.get('http://fake-graphkb/api/version') + second = conn.http.get('http://fake-graphkb/api/version') + + assert adapter.calls == 1 + assert first.json() == second.json() + assert getattr(second, 'from_cache', False) is True + + def test_no_cache_header_bypasses_cache(self): + conn = GraphKBConnection(url='http://fake-graphkb', use_global_cache=True) + adapter = CountingAdapter() + conn.http.mount('http://', adapter) + + conn.http.get('http://fake-graphkb/api/version') + conn.http.get('http://fake-graphkb/api/version', headers={'Cache-Control': 'no-cache'}) + + assert adapter.calls == 2 + + def test_only_if_cached_returns_504_when_not_cached(self): + conn = GraphKBConnection(url='http://fake-graphkb', use_global_cache=True) + adapter = CountingAdapter() + conn.http.mount('http://', adapter) + + resp = conn.http.get( + 'http://fake-graphkb/api/never-requested', + headers={'Cache-Control': 'only-if-cached'}, + ) + + assert resp.status_code == 504 + assert adapter.calls == 0 + + def test_only_if_cached_returns_cached_value_without_new_request(self): + conn = GraphKBConnection(url='http://fake-graphkb', use_global_cache=True) + adapter = CountingAdapter() + conn.http.mount('http://', adapter) + + conn.http.get('http://fake-graphkb/api/version') + resp = conn.http.get( + 'http://fake-graphkb/api/version', headers={'Cache-Control': 'only-if-cached'} + ) + + assert resp.status_code == 200 + assert adapter.calls == 1 + + def test_post_to_query_endpoint_is_cached(self): + conn = GraphKBConnection(url='http://fake-graphkb', use_global_cache=True) + adapter = CountingAdapter() + conn.http.mount('http://', adapter) + + conn.http.post('http://fake-graphkb/api/query', data='{}') + conn.http.post('http://fake-graphkb/api/query', data='{}') + + assert adapter.calls == 1 + + def test_post_to_non_query_endpoint_is_never_cached(self): + conn = GraphKBConnection(url='http://fake-graphkb', use_global_cache=True) + adapter = CountingAdapter() + conn.http.mount('http://', adapter) + + conn.http.post('http://fake-graphkb/api/statement', data='{}') + conn.http.post('http://fake-graphkb/api/statement', data='{}') + + assert adapter.calls == 2 + + def test_use_global_cache_false_hits_adapter_every_time(self): + conn = GraphKBConnection(url='http://fake-graphkb', use_global_cache=False) + adapter = CountingAdapter() + conn.http.mount('http://', adapter) + + conn.http.get('http://fake-graphkb/api/version') + conn.http.get('http://fake-graphkb/api/version') + + assert adapter.calls == 2 + + def test_use_global_cache_false_raises_with_cache_name(self): + with pytest.raises(NotImplementedError): + GraphKBConnection( + url='http://fake-graphkb', use_global_cache=False, cache_name='somewhere' + ) diff --git a/tests/test_ipr/test_annotate.py b/tests/test_ipr/test_annotate.py index 43de4216..79eb54c7 100644 --- a/tests/test_ipr/test_annotate.py +++ b/tests/test_ipr/test_annotate.py @@ -6,6 +6,7 @@ from pori_python.ipr.constants import ( COSMIC_SIGNATURE_VARIANT_TYPE, HLA_SIGNATURE_VARIANT_TYPE, + HRD_MAPPING, MSI_MAPPING, TMB_SIGNATURE, TMB_SIGNATURE_VARIANT_TYPE, @@ -13,7 +14,7 @@ from pori_python.ipr.inputs import preprocess_signature_variants from pori_python.types import IprSmallMutationVariant -from .test_ipr import DISEASE_RIDS +from .test_ipr import DISEASE_RIDS # dummy RIDs; will always result in matchedCancer == false EXCLUDE_BCGSC_TESTS = os.environ.get('EXCLUDE_BCGSC_TESTS') == '1' @@ -203,6 +204,29 @@ def test_annotate_signature_variants_msi(self, graphkb_conn): ) assert len(msi) != 0 + def test_annotate_signature_variants_hrd_strong(self, graphkb_conn): + """Test HRD strong signature""" + hrd = annotate_signature_variants( + graphkb_conn, + DISEASE_RIDS, + preprocess_signature_variants( + [HRD_MAPPING['homologous recombination deficiency strong signature']] + ), + ) + assert len(hrd) != 0 + + @pytest.mark.skip(reason='no GKB statement for HRD moderate Signature CVs yet') + def test_annotate_signature_variants_hrd_moderate(self, graphkb_conn): + """Test HRD moderate signature""" + hrd = annotate_signature_variants( + graphkb_conn, + DISEASE_RIDS, + preprocess_signature_variants( + [HRD_MAPPING['homologous recombination deficiency moderate signature']] + ), + ) + assert len(hrd) != 0 + def test_annotate_structural_variants_tp53(self, graphkb_conn): """Verify alternate TP53 variants match.""" ref_key = 'prot_only' diff --git a/tests/test_ipr/test_ipr.py b/tests/test_ipr/test_ipr.py index 687c14b4..ba270153 100644 --- a/tests/test_ipr/test_ipr.py +++ b/tests/test_ipr/test_ipr.py @@ -1034,3 +1034,9 @@ def test_create_key_alterations_includes_only_pruned_kbmatches(self): # partial matches is false assert len(key_alts1) == 3 assert len(key_alts2) == 4 + assert {(alt['variantType'], alt['variant']) for alt in key_alts1} == { + (variant['variantType'], variant['key']) for variant in ALL_VARIANTS[:3] + } + assert {(alt['variantType'], alt['variant']) for alt in key_alts2} == { + (variant['variantType'], variant['key']) for variant in ALL_VARIANTS[:4] + } diff --git a/tests/test_ipr/test_ipr_integration.py b/tests/test_ipr/test_ipr_integration.py new file mode 100644 index 00000000..af9c5793 --- /dev/null +++ b/tests/test_ipr/test_ipr_integration.py @@ -0,0 +1,102 @@ +import os +import json +import jsonschema +import pytest + +from pori_python.ipr.connection import IprConnection + +from .constants import EXCLUDE_INTEGRATION_TESTS + + +mock_ipr_results = [ + [ + { + 'text': '

no cancerType

', + 'variantName': 'ERBB2 amplification', + 'cancerType': [], + 'template': {'name': 'test3'}, + 'projects': [{'name': 'test2'}], + }, + { + 'text': '

normal

', + 'variantName': 'ERBB2 amplification', + 'cancerType': ['test1', 'test'], + 'template': {'name': 'test3'}, + 'projects': [{'name': 'test2'}], + }, + { + 'text': '

no project

', + 'variantName': 'ERBB2 amplification', + 'cancerType': ['test1', 'test'], + 'template': {'name': 'test3'}, + }, + { + 'text': '

no template

', + 'variantName': 'ERBB2 amplification', + 'cancerType': ['test1', 'test'], + 'projects': [{'name': 'test2'}], + }, + ], + [ + { + 'text': '

normal, second variant

', + 'variantName': 'second variant', + 'cancerType': ['test1', 'test'], + 'template': {'name': 'test3'}, + 'projects': [{'name': 'test2'}], + }, + ], +] + + +def validate_mock_ipr_results_against_schema(schema, results): + # Since the IPR generated schema has required fields, but the purpose of the test is to prevent + # schema drift and not to validate POST payloads to variant-text, we create a custom validator + # that ignores the required fields and only validates the properties that are defined in the schema. + custom_validators = dict(jsonschema.Draft7Validator.VALIDATORS) + custom_validators.pop('required', None) + non_required_validator = jsonschema.validators.create( + meta_schema=jsonschema.Draft7Validator.META_SCHEMA, + validators=custom_validators, + version='draft7-lazy', + ) + validator = non_required_validator(schema) + declared_properties = set(schema.get('properties', {})) + + for group_index, group in enumerate(results): + for record_index, record in enumerate(group): + location = f'group {group_index} record {record_index}' + unknown = sorted(set(record) - declared_properties) + assert not unknown, ( + f'{location}: properties {unknown} are not defined in the IPR variant-text schema' + ) + errors = sorted(validator.iter_errors(record), key=lambda err: list(err.path)) + assert not errors, f'{location}: ' + '; '.join( + f'{list(err.path)}: {err.message}' for err in errors + ) + + +def make_ipr_connection(): + return IprConnection( + username=os.environ.get('IPR_USER', os.environ['USER']), + password=os.environ['IPR_PASS'], + url=os.environ['IPR_INTEGRATION_TEST_URL'], + ) + + +# DEVSU-3011 adding IPR integration test to validate variant text schema from the IPR API side to prevent schema drift +@pytest.mark.skipif(EXCLUDE_INTEGRATION_TESTS, reason='excluding long running integration tests') +class TestVariantTextSchema: + def test_mock_ipr_results_match_variant_text_schema(self): + ipr_conn = make_ipr_connection() + schema = ipr_conn.request( + 'variant-text/schema', + method='GET', + headers=json.dumps( + { + 'Content-Length': '0', + 'Accept': 'application/json', + } + ), + ) + validate_mock_ipr_results_against_schema(schema, mock_ipr_results) diff --git a/tests/test_ipr/test_main.py b/tests/test_ipr/test_main.py index 3dbd0aa5..1dc20ab2 100644 --- a/tests/test_ipr/test_main.py +++ b/tests/test_ipr/test_main.py @@ -151,7 +151,6 @@ def side_effect_function(*args, **kwargs): return report_content -@pytest.mark.skip(reason='KBDEV-1308; taking too long, getting canceled after reaching max delay') @pytest.mark.skipif(EXCLUDE_INTEGRATION_TESTS, reason='excluding long running integration tests') class TestCreateReport: def test_main_sections_present(self, report_upload_content: Dict) -> None: diff --git a/tests/test_ipr/test_summary.py b/tests/test_ipr/test_summary.py index 248a99f2..c3e128f6 100644 --- a/tests/test_ipr/test_summary.py +++ b/tests/test_ipr/test_summary.py @@ -166,14 +166,14 @@ def test_multiple_diseases_only_matches(self): 'variantName': 'ERBB2 amplification', 'cancerType': [], 'template': {'name': 'test3'}, - 'project': {'name': 'test2'}, + 'projects': [{'name': 'test2'}], }, { 'text': '

normal

', 'variantName': 'ERBB2 amplification', 'cancerType': ['test1', 'test'], 'template': {'name': 'test3'}, - 'project': {'name': 'test2'}, + 'projects': [{'name': 'test2'}], }, { 'text': '

no project

', @@ -185,7 +185,7 @@ def test_multiple_diseases_only_matches(self): 'text': '

no template

', 'variantName': 'ERBB2 amplification', 'cancerType': ['test1', 'test'], - 'project': {'name': 'test2'}, + 'projects': [{'name': 'test2'}], }, ], [ @@ -194,7 +194,7 @@ def test_multiple_diseases_only_matches(self): 'variantName': 'second variant', 'cancerType': ['test1', 'test'], 'template': {'name': 'test3'}, - 'project': {'name': 'test2'}, + 'projects': [{'name': 'test2'}], }, ], ] @@ -204,10 +204,10 @@ def test_multiple_diseases_only_matches(self): class TestVariantTextFromIPR: def test_gets_fully_matched_output_when_possible(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) matches = [{'kbVariant': 'ERBB2 amplification'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, disease_name='test1', disease_match_names=[], @@ -218,15 +218,15 @@ def test_gets_fully_matched_output_when_possible(self): include_nonspecific_template=True, ) summary_lines = ipr_summary.split('\n') - assert summary_lines[1] == '

ERBB2 amplification (test1,test)

' + assert summary_lines[1] == '

ERBB2 amplification (test1, test)

' assert summary_lines[2] == '

normal

' assert len(summary_lines) == 3 def test_omits_nonspecific_project_matches_when_specified(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) matches = [{'kbVariant': 'ERBB2 amplification'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, disease_name='test1', disease_match_names=[], @@ -239,10 +239,10 @@ def test_omits_nonspecific_project_matches_when_specified(self): assert ipr_summary == no_comments_found_output def test_omits_nonspecific_template_matches_when_specified(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) matches = [{'kbVariant': 'ERBB2 amplification'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, disease_name='test1', disease_match_names=[], @@ -255,10 +255,10 @@ def test_omits_nonspecific_template_matches_when_specified(self): assert ipr_summary == no_comments_found_output def test_omits_nonspecific_disease_matches_when_specified(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) matches = [{'kbVariant': 'ERBB2 amplification'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, disease_name='notfound', disease_match_names=[], @@ -271,10 +271,10 @@ def test_omits_nonspecific_disease_matches_when_specified(self): assert ipr_summary == no_comments_found_output def test_includes_nonspecific_project_matches_when_specified(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) matches = [{'kbVariant': 'ERBB2 amplification'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, disease_name='test1', disease_match_names=[], @@ -289,10 +289,10 @@ def test_includes_nonspecific_project_matches_when_specified(self): assert len(summary_lines) == 3 def test_includes_nonspecific_template_matches_when_specified(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) matches = [{'kbVariant': 'ERBB2 amplification'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, disease_name='test1', disease_match_names=[], @@ -307,10 +307,10 @@ def test_includes_nonspecific_template_matches_when_specified(self): assert len(summary_lines) == 3 def test_includes_nonspecific_disease_matches_when_specified(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) matches = [{'kbVariant': 'ERBB2 amplification'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, disease_name='notfound', disease_match_names=[], @@ -321,15 +321,15 @@ def test_includes_nonspecific_disease_matches_when_specified(self): include_nonspecific_template=False, ) summary_lines = ipr_summary.split('\n') - assert summary_lines[1] == '

ERBB2 amplification (no specific cancer types)

' + assert summary_lines[1] == '

ERBB2 amplification (no specific cancer types)

' assert summary_lines[2] == '

no cancerType

' assert len(summary_lines) == 3 def test_includes_all_graphkb_disease_matches(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) matches = [{'kbVariant': 'ERBB2 amplification'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, disease_name='notfound', disease_match_names=['TEST1'], @@ -340,19 +340,19 @@ def test_includes_all_graphkb_disease_matches(self): include_nonspecific_template=False, ) summary_lines = ipr_summary.split('\n') - assert summary_lines[1] == '

ERBB2 amplification (test1,test)

' + assert summary_lines[1] == '

ERBB2 amplification (test1, test)

' assert summary_lines[2] == '

normal

' assert len(summary_lines) == 3 def test_prepare_section_for_multiple_variants(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) # NB this test relies on matches being processed in this order matches = [{'kbVariant': 'ERBB2 amplification'}, {'kbVariant': 'second variant'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=copy(mock_ipr_results))) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, - disease_name='test1', - disease_match_names=[], + disease_name='notfound', + disease_match_names=['TEST1'], project_name='test2', report_type='test3', include_nonspecific_project=False, @@ -363,17 +363,17 @@ def test_prepare_section_for_multiple_variants(self): assert len(summary_lines) == 5 assert ( '\n'.join(summary_lines[1:]) - == '

ERBB2 amplification (test1,test)

\n

normal

\n

second variant (test1,test)

\n

normal, second variant

' + == '

ERBB2 amplification (test1, test)

\n

normal

\n

second variant (test1, test)

\n

normal, second variant

' ) def test_empty_section_when_no_variant_match(self): - ipr_conn = MagicMock(get=MagicMock(side_effect=[[], []])) matches = [{'kbVariant': 'notfound1'}, {'kbVariant': 'notfound2'}] + mock_ipr_conn = MagicMock(get=MagicMock(side_effect=[[], []])) ipr_summary = get_ipr_analyst_comments( - ipr_conn, + mock_ipr_conn, matches=matches, - disease_name='test1', - disease_match_names=[], + disease_name='notfound', + disease_match_names=['TEST1'], project_name='test2', report_type='test3', include_nonspecific_project=False, diff --git a/tests/test_ipr/test_upload.py b/tests/test_ipr/test_upload.py index 6e7f9b05..4445b9c4 100644 --- a/tests/test_ipr/test_upload.py +++ b/tests/test_ipr/test_upload.py @@ -189,7 +189,7 @@ def loaded_reports(tmp_path_factory) -> Generator: '--graphkb_password', os.environ.get('GRAPHKB_PASS', os.environ['IPR_PASS']), '--ipr_url', - os.environ['IPR_TEST_URL'], + os.environ['IPR_URL'], '--graphkb_url', os.environ.get('GRAPHKB_URL', False), '--therapeutics', @@ -213,7 +213,7 @@ def loaded_reports(tmp_path_factory) -> Generator: ipr_conn = IprConnection( username=os.environ.get('IPR_USER', os.environ['USER']), password=os.environ['IPR_PASS'], - url=os.environ['IPR_TEST_URL'], + url=os.environ['IPR_URL'], ) loaded_report = ipr_conn.get(uri=f'reports?searchText={patient_id}') async_loaded_report = ipr_conn.get(uri=f'reports?searchText={async_patient_id}') @@ -233,7 +233,7 @@ def get_section(loaded_report, section_name): ipr_conn = IprConnection( username=os.environ.get('IPR_USER', os.environ['USER']), password=os.environ['IPR_PASS'], - url=os.environ['IPR_TEST_URL'], + url=os.environ['IPR_URL'], ) return ipr_conn.get(uri=f'reports/{ident}/{section_name}')