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 checking for registry path #483

Merged
merged 9 commits into from
Mar 20, 2024
39 changes: 23 additions & 16 deletions looper/cli_pydantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,14 @@
from .utils import (
dotfile_path,
enrich_args_via_cfg,
is_registry_path,
is_pephub_registry_path,
read_looper_config_file,
read_looper_dotfile,
initiate_looper_config,
init_generic_pipeline,
read_yaml_file,
inspect_looper_config_file,
is_PEP_file_type,
)

from typing import List, Tuple
Expand Down Expand Up @@ -176,41 +177,47 @@ def run_looper(args: TopLevelParser, parser: ArgumentParser, test_args=None):
subcommand_args.ignore_flags = True

# Initialize project
if is_registry_path(subcommand_args.config_file):
if vars(subcommand_args)[SAMPLE_PL_ARG]:
if is_PEP_file_type(subcommand_args.config_file) and os.path.exists(
subcommand_args.config_file
):
try:
p = Project(
cfg=subcommand_args.config_file,
amendments=subcommand_args.amend,
divcfg_path=divcfg,
runp=subcommand_name == "runp",
project_dict=PEPHubClient()._load_raw_pep(
registry_path=subcommand_args.config_file
),
**{
attr: getattr(subcommand_args, attr)
for attr in CLI_PROJ_ATTRS
if attr in subcommand_args
},
)
else:
raise MisconfigurationException(
f"`sample_pipeline_interface` is missing. Provide it in the parameters."
)
else:
try:
except yaml.parser.ParserError as e:
_LOGGER.error(f"Project config parse failed -- {e}")
sys.exit(1)
elif is_pephub_registry_path(subcommand_args.config_file):
if vars(subcommand_args)[SAMPLE_PL_ARG]:
p = Project(
cfg=subcommand_args.config_file,
amendments=subcommand_args.amend,
divcfg_path=divcfg,
runp=subcommand_name == "runp",
project_dict=PEPHubClient()._load_raw_pep(
registry_path=subcommand_args.config_file
),
**{
attr: getattr(subcommand_args, attr)
for attr in CLI_PROJ_ATTRS
if attr in subcommand_args
},
)
except yaml.parser.ParserError as e:
_LOGGER.error(f"Project config parse failed -- {e}")
sys.exit(1)
else:
raise MisconfigurationException(
f"`sample_pipeline_interface` is missing. Provide it in the parameters."
)
else:
raise MisconfigurationException(
f"Cannot load PEP. Check file path or registry path to pep."
)

selected_compute_pkg = p.selected_compute_package or DEFAULT_COMPUTE_RESOURCES_NAME
if p.dcc is not None and not p.dcc.activate_package(selected_compute_pkg):
Expand Down
38 changes: 26 additions & 12 deletions looper/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ def initiate_looper_config(
return False

if pep_path:
if is_registry_path(pep_path):
if is_pephub_registry_path(pep_path):
pass
else:
pep_path = expandpath(pep_path)
Expand Down Expand Up @@ -562,10 +562,20 @@ def read_looper_config_file(looper_config_path: str) -> dict:
for k, v in return_dict.items():
if isinstance(v, str):
v = expandpath(v)
if not os.path.isabs(v) and not is_registry_path(v):
return_dict[k] = os.path.join(config_dir_path, v)
else:
# TODO this is messy because is_pephub_registry needs to fail on anything NOT a pephub registry path
# https://github.com/pepkit/ubiquerg/issues/43
if is_PEP_file_type(v):
if not os.path.isabs(v):
return_dict[k] = os.path.join(config_dir_path, v)
else:
return_dict[k] = v
elif is_pephub_registry_path(v):
return_dict[k] = v
else:
if not os.path.isabs(v):
return_dict[k] = os.path.join(config_dir_path, v)
else:
return_dict[k] = v

return return_dict

Expand Down Expand Up @@ -598,19 +608,23 @@ def dotfile_path(directory=os.getcwd(), must_exist=False):
cur_dir = parent_dir


def is_registry_path(input_string: str) -> bool:
def is_PEP_file_type(input_string: str) -> bool:
"""
Determines if the provided path is actually a file type that Looper can use for loading PEP
"""

PEP_FILE_TYPES = ["yaml", "csv"]

res = list(filter(input_string.endswith, PEP_FILE_TYPES)) != []
return res


def is_pephub_registry_path(input_string: str) -> bool:
"""
Check if input is a registry path to pephub
:param str input_string: path to the PEP (or registry path)
:return bool: True if input is a registry path
"""
try:
if input_string.endswith(".yaml"):
return False
except AttributeError:
raise RegistryPathException(
msg=f"Malformed registry path. Unable to parse {input_string} as a registry path."
)
try:
registry_path = RegistryPath(**parse_registry_path(input_string))
except (ValidationError, TypeError):
Expand Down
37 changes: 37 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,24 @@ def prep_temp_pep_basic(example_pep_piface_path):
return path_to_looper_config


@pytest.fixture
def prep_temp_pep_csv(example_pep_piface_path):

# Get Path to local copy of hello_looper
hello_looper_dir_path = os.path.join(
example_pep_piface_path, "hello_looper-dev_derive"
)

# Make local temp copy of hello_looper
d = tempfile.mkdtemp()
shutil.copytree(hello_looper_dir_path, d, dirs_exist_ok=True)

advanced_dir = os.path.join(d, "csv")
path_to_looper_config = os.path.join(advanced_dir, ".looper.yaml")

return path_to_looper_config


@pytest.fixture
def prep_temp_config_with_pep(example_pep_piface_path):
# temp dir
Expand Down Expand Up @@ -287,3 +305,22 @@ def prep_temp_pep_pipestat_advanced(example_pep_piface_path):
path_to_looper_config = os.path.join(advanced_dir, ".looper_advanced_pipestat.yaml")

return path_to_looper_config


@pytest.fixture
def prep_temp_pep_pephub(example_pep_piface_path):

# Get Path to local copy of hello_looper

hello_looper_dir_path = os.path.join(
example_pep_piface_path, "hello_looper-dev_derive"
)

# Make local temp copy of hello_looper
d = tempfile.mkdtemp()
shutil.copytree(hello_looper_dir_path, d, dirs_exist_ok=True)

advanced_dir = os.path.join(d, "pephub")
path_to_looper_config = os.path.join(advanced_dir, ".looper.yaml")

return path_to_looper_config
5 changes: 2 additions & 3 deletions tests/smoketests/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
import pytest
from peppy import Project

from looper.const import FLAGS
from looper.exceptions import PipestatConfigurationException
from looper.exceptions import PipestatConfigurationException, MisconfigurationException
from tests.conftest import *
from looper.cli_pydantic import main
import pandas as pd
Expand Down Expand Up @@ -607,5 +606,5 @@ def test_inspect_config(self, prep_temp_pep, cmd):
def test_inspect_no_config_found(self, cmd):
"Checks inspect command"
x = [cmd]
with pytest.raises(ValueError):
with pytest.raises(MisconfigurationException):
results = main(test_args=x)
48 changes: 33 additions & 15 deletions tests/smoketests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,26 @@ def test_cli(prep_temp_pep):
raise pytest.fail("DID RAISE {0}".format(Exception))


@pytest.mark.skip(reason="PEP via CSV is currently broken.")
def test_running_csv_pep(prep_temp_pep_csv):
tp = prep_temp_pep_csv

x = ["run", "--looper-config", tp, "--dry-run"]
try:
main(test_args=x)
except Exception:
raise pytest.fail("DID RAISE {0}".format(Exception))


@pytest.mark.parametrize(
"path", ["something/example.yaml", "somethingelse/example2.csv"]
)
def test_is_PEP_file_type(path):

result = is_PEP_file_type(path)
assert result == True


def is_connected():
"""Determines if local machine can connect to the internet."""
import socket
Expand Down Expand Up @@ -54,7 +74,7 @@ def test_looper_cfg_required(self, cmd):

x = test_args_expansion("", cmd)

with pytest.raises(ValueError):
with pytest.raises(MisconfigurationException):
ff = main(test_args=x)
print(ff)

Expand Down Expand Up @@ -574,20 +594,7 @@ class TestLooperPEPhub:
],
)
def test_pephub_registry_path_recognition(self, pep_path):
assert is_registry_path(pep_path) is True

@pytest.mark.parametrize(
"pep_path",
[
"some/path/to/pep.yaml",
"different/path.yaml",
"default/path/to/file/without/yaml",
"file_in_folder.yaml",
"not_yaml_file",
],
)
def test_config_recognition(self, pep_path):
assert is_registry_path(pep_path) is False
assert is_pephub_registry_path(pep_path) is True

def test_init_project_using_dict(self, prep_temp_config_with_pep):
"""Verify looper runs using pephub in a basic case and return code is 0"""
Expand All @@ -597,3 +604,14 @@ def test_init_project_using_dict(self, prep_temp_config_with_pep):
)

assert len(init_project.pipeline_interfaces) == 3

def test_init_project_using_csv(self, prep_temp_pep_csv):
"""Verify looper runs using pephub in a basic case and return code is 0"""
tp = prep_temp_pep_csv
with mod_yaml_data(tp) as config_data:
pep_config_csv = config_data["pep_config"]

pep_config_csv = os.path.join(os.path.dirname(tp), pep_config_csv)
init_project = Project(cfg=pep_config_csv)

assert len(init_project.samples) == 2
13 changes: 13 additions & 0 deletions tests/test_comprehensive.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,16 @@ def test_comprehensive_looper_pipestat(prep_temp_pep_pipestat):
assert len(tsv_list) == 0
with pytest.raises(RecordNotFoundError):
retrieved_result = psm.retrieve_one(record_identifier="frog_2")


def test_comprehensive_looper_pephub(prep_temp_pep_pephub):
"""Basic test to determine if Looper can run a PEP from PEPHub"""

path_to_looper_config = prep_temp_pep_pephub

x = ["run", "--looper-config", path_to_looper_config]

try:
results = main(test_args=x)
except Exception:
raise pytest.fail("DID RAISE {0}".format(Exception))
Loading