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

Store demo case file paths as absolute paths #4517

Merged
merged 21 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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 @@ -11,6 +11,7 @@ About changelog [here](https://keepachangelog.com/en/1.0.0/)
- All links in disease table on diagnosis page now open in a new tab
- Dark mode settings applied to multiselects on institute settings
- Comments on case and variant pages can be viewed by expanding an accordion
- Demo case file paths are now stored as absolute paths
### Fixed
- On variants page, search for variants in genes present only in build 38 returning no results
- Pin/unpin with API was not able to make event links
Expand Down
42 changes: 21 additions & 21 deletions scout/models/case/case_loading_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from enum import Enum
from fractions import Fraction
from glob import glob
from os.path import abspath, exists, isabs
from pathlib import Path
from typing import Any, Dict, List, Optional, Tuple, Union

Expand All @@ -12,7 +13,6 @@
except ImportError:
from typing_extensions import Literal

import path
from pydantic import BaseModel, Field, field_validator, model_validator

from scout.constants import ANALYSIS_TYPES
Expand Down Expand Up @@ -49,7 +49,11 @@
"peddy_ped",
"peddy_ped_check",
"peddy_sex_check",
"exe_ver",
"smn_tsv",
"reference_info",
"RNAfusion_inspector",
"RNAfusion_inspector_research",
"RNAfusion_report",
"RNAfusion_report_research",
]
Expand Down Expand Up @@ -86,18 +90,14 @@ class Sex(str, Enum):
unknown = "unknown"


def _get_demo_file_absolute_path(partial_path: str) -> str:
"""returns the absolute path to a given demo file."""
APP_ROOT: str = path.abspath(path.join(path.dirname(__file__), ".."))
return path.join(APP_ROOT, partial_path)


def _is_string_path(string_path: str) -> bool:
try:
path = Path(string_path) or Path(_get_demo_file_absolute_path(partial_path=string_path))
return path.is_file()
except AttributeError:
return False
def _resource_abs_path(string_path: str) -> str:
"""Return the absolute path to a resource file."""
if exists(string_path) and isabs(string_path):
Copy link
Collaborator

Choose a reason for hiding this comment

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

you could flatten this conditional as

Suggested change
if exists(string_path) and isabs(string_path):
if not exists(string_path):
raise FileExistsError(f"Path {string_path} could not be found on disk.")
if isabs(string_path):
return string_path
return abspath(string_path)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I like it!

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

return string_path
elif exists(string_path):
return abspath(string_path)
else:
raise FileExistsError(f"File {string_path} could not be found on disk.")


#### VCF files class ####
Expand Down Expand Up @@ -125,9 +125,8 @@ def validate_file_path(cls, values: Dict) -> "VcfFiles":
"""Make sure that VCF file exists on disk."""
for item in VCF_FILE_PATH_CHECKS:
item_path: str = values.get(item)
if item_path and _is_string_path(values[item]) is False:
raise ValueError(f"{item} path '{values[item]}' is not valid.")

if item_path:
values[item] = _resource_abs_path(item_path)
return values


Expand Down Expand Up @@ -246,8 +245,8 @@ def validate_file_path(cls, values: Dict) -> "SampleLoader":
"""Make sure that files associated to samples (mostly alignment files) exist on disk."""
for item in SAMPLES_FILE_PATH_CHECKS:
item_path: str = values.get(item)
if item_path and _is_string_path(values[item]) is False:
northwestwitch marked this conversation as resolved.
Show resolved Hide resolved
raise ValueError(f"{item} path '{values[item]}' is not valid.")
if item_path:
values[item] = _resource_abs_path(item_path)

return values

Expand Down Expand Up @@ -283,7 +282,7 @@ def check_image_path(cls, path: str) -> Optional[str]:
raise TypeError(
f"Custom images should be of type: {', '.join(SUPPORTED_IMAGE_FORMATS)}"
)
if REPID not in path and _is_string_path(path) is False:
if REPID not in path and exists(path) is False:
raise ValueError(f"Image path '{path}' is not valid.")
return path

Expand Down Expand Up @@ -387,6 +386,7 @@ class CaseLoader(BaseModel):
peddy_sex_check: Optional[str] = Field(None, alias="peddy_sex") # Soon to be deprecated
phenotype_groups: Optional[List[str]] = None
phenotype_terms: Optional[List[str]] = None
exe_ver: Optional[str] = None
rank_model_version: Optional[str] = None
rank_score_threshold: Optional[int] = 0
reference_info: Optional[str] = None
Expand Down Expand Up @@ -502,8 +502,8 @@ def validate_file_path(cls, values: Dict) -> "CaseLoader":
"""Make sure the files associated to the case (mostly reports) exist on disk."""
for item in CASE_FILE_PATH_CHECKS:
item_path: str = values.get(item)
if item_path and _is_string_path(values[item]) is False:
raise ValueError(f"{item} path '{values[item]}' is not valid.")
if item_path:
values[item] = _resource_abs_path(item_path)

return values

Expand Down
10 changes: 6 additions & 4 deletions tests/parse/test_parse_case.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
from datetime import datetime
from os import path

import pytest
from pydantic import ValidationError
Expand Down Expand Up @@ -73,8 +74,8 @@ def test_parse_case_parsing(scout_config, param_name):
# GIVEN you load sample information from a scout config
# WHEN case is parsed
case_data = parse_case_config(scout_config)
# THEN the case should have the parameter
assert case_data[param_name] == scout_config[param_name]
# THEN the case should have the parameter, either as string of full path to a resource
assert case_data[param_name] == scout_config[param_name] or path.exists(case_data[param_name])
northwestwitch marked this conversation as resolved.
Show resolved Hide resolved


@pytest.mark.parametrize(
Expand Down Expand Up @@ -193,8 +194,9 @@ def test_parse_case_vcf_files(scout_config, vcf_file):
# GIVEN you load sample information from a scout config
# WHEN case is parsed
case_data = parse_case_config(scout_config)
# THEN the case should the same vcf files as specified in the
assert case_data["vcf_files"][vcf_file] == scout_config[vcf_file]

# THEN the case should contain the absolute path to the resources specified in the case config
assert path.exists(case_data["vcf_files"][vcf_file])


@pytest.mark.parametrize("bam_name", ["alignment_path", "bam_file", "bam_path"])
Expand Down
Loading