[ENH]: Specify which shell to use when using junifer queue #273

Merged
synchon merged 10 commits from feat/queue-shell into main 2024-04-09 11:51:47 +00:00
15 changed files with 176 additions and 50 deletions

View file

@ -0,0 +1 @@
Add support for choosing between ``bash`` and ``zsh`` shells when queueing by `Synchon Mandal`_

View file

@ -1 +0,0 @@
Add a validation step on the :func:`.run` function to validate the marker collection by `Fede Raimondo`_

View file

@ -1 +0,0 @@
Add the executable flag to the ants docker scripts, fsl docker scripts and other running scripts by `Fede Raimondo_`

View file

@ -46,6 +46,10 @@ Bugfixes
(:gh:`312`)
- Fix element access for :class:`.DMCC13Benchmark` DataGrabber by `Synchon
Mandal`_ (:gh:`314`)
- Add a validation step on the :func:`.run` function to validate the marker
collection by `Fede Raimondo`_ (:gh:`320`)
- Add the executable flag to the ants docker scripts, fsl docker scripts and
other running scripts by `Fede Raimondo`_ (:gh:`321`)
- Force ``str`` dtype when parsing elements from file by `Synchon Mandal`_
(:gh:`322`)

View file

@ -43,7 +43,8 @@ class GnuParallelLocalAdapter(QueueContextAdapter):
Raises
------
ValueError
If``env`` is invalid.
If ``env.kind`` is invalid or
if ``env.shell`` is invalid.
See Also
--------
@ -110,13 +111,22 @@ class GnuParallelLocalAdapter(QueueContextAdapter):
f"must be one of {valid_env_kinds}"
)
else:
# Check shell
shell = env.get("shell", "bash")
valid_shells = ["bash", "zsh"]
if shell not in valid_shells:
raise_error(
f"Invalid value for `env.shell`: {shell}, "
f"must be one of {valid_shells}"
)
self._shell = shell
# Set variables
if env["kind"] == "local":
# No virtual environment
self._executable = "junifer"
self._arguments = ""
else:
self._executable = f"run_{env['kind']}.sh"
self._executable = f"run_{env['kind']}.{self._shell}"
self._arguments = f"{env['name']} junifer"
self._exec_path = self._job_dir / self._executable
@ -135,7 +145,7 @@ class GnuParallelLocalAdapter(QueueContextAdapter):
def pre_run(self) -> str:
"""Return pre-run commands."""
fixed = (
"#!/usr/bin/env bash\n\n"
f"#!/usr/bin/env {self._shell}\n\n"
"# This script is auto-generated by junifer.\n\n"
"# Force datalad to run in non-interactive mode\n"
"DATALAD_UI_INTERACTIVE=false\n"
@ -146,7 +156,7 @@ class GnuParallelLocalAdapter(QueueContextAdapter):
def run(self) -> str:
"""Return run commands."""
return (
"#!/usr/bin/env bash\n\n"
f"#!/usr/bin/env {self._shell}\n\n"
"# This script is auto-generated by junifer.\n\n"
"# Run pre_run.sh\n"
f"sh {self._pre_run_path.resolve()!s}\n\n"
@ -166,7 +176,7 @@ class GnuParallelLocalAdapter(QueueContextAdapter):
def pre_collect(self) -> str:
"""Return pre-collect commands."""
fixed = (
"#!/usr/bin/env bash\n\n"
f"#!/usr/bin/env {self._shell}\n\n"
"# This script is auto-generated by junifer.\n"
)
var = self._pre_collect or ""
@ -175,7 +185,7 @@ class GnuParallelLocalAdapter(QueueContextAdapter):
def collect(self) -> str:
"""Return collect commands."""
return (
"#!/usr/bin/env bash\n\n"
f"#!/usr/bin/env {self._shell}\n\n"
"# This script is auto-generated by junifer.\n\n"
"# Run pre_collect.sh\n"
f"sh {self._pre_collect_path.resolve()!s}\n\n"

View file

@ -126,7 +126,8 @@ class HTCondorAdapter(QueueContextAdapter):
Raises
------
ValueError
If ``env.kind`` is invalid.
If ``env.kind`` is invalid or
if ``env.shell`` is invalid.
"""
# Set env related variables
@ -140,13 +141,22 @@ class HTCondorAdapter(QueueContextAdapter):
f"must be one of {valid_env_kinds}"
)
else:
# Check shell
shell = env.get("shell", "bash")
valid_shells = ["bash", "zsh"]
if shell not in valid_shells:
raise_error(
f"Invalid value for `env.shell`: {shell}, "
f"must be one of {valid_shells}"
)
self._shell = shell
# Set variables
if env["kind"] == "local":
# No virtual environment
self._executable = "junifer"
self._arguments = ""
else:
self._executable = f"run_{env['kind']}.sh"
self._executable = f"run_{env['kind']}.{self._shell}"
self._arguments = f"{env['name']} junifer"
self._exec_path = self._job_dir / self._executable
@ -181,7 +191,7 @@ class HTCondorAdapter(QueueContextAdapter):
def pre_run(self) -> str:
"""Return pre-run commands."""
fixed = (
"#!/bin/bash\n\n"
f"#!/usr/bin/env {self._shell}\n\n"
"# This script is auto-generated by junifer.\n\n"
"# Force datalad to run in non-interactive mode\n"
"DATALAD_UI_INTERACTIVE=false\n"
@ -225,12 +235,13 @@ class HTCondorAdapter(QueueContextAdapter):
def pre_collect(self) -> str:
"""Return pre-collect commands."""
fixed = (
"#!/bin/bash\n\n" "# This script is auto-generated by junifer.\n"
f"#!/usr/bin/env {self._shell}\n\n"
"# This script is auto-generated by junifer.\n"
)
var = self._pre_collect or ""
# Add commands if collect="yes"
if self._collect == "yes":
var += 'if [ "${1}" == "4" ]; then\n' " exit 1\n" "fi\n"
var += 'if [ "${1}" == "4" ]; then\n exit 1\nfi\n'
return fixed + "\n" + var
def collect(self) -> str:

View file

@ -12,11 +12,11 @@ import pytest
from junifer.api.queue_context import GnuParallelLocalAdapter
def test_GnuParallelLocalAdapter_env_error() -> None:
def test_GnuParallelLocalAdapter_env_kind_error() -> None:
"""Test error for invalid env kind."""
with pytest.raises(ValueError, match="Invalid value for `env.kind`"):
GnuParallelLocalAdapter(
job_name="check_env",
job_name="check_env_kind",
job_dir=Path("."),
yaml_config_path=Path("."),
elements=["sub01"],
@ -24,6 +24,18 @@ def test_GnuParallelLocalAdapter_env_error() -> None:
)
def test_GnuParallelLocalAdapter_env_shell_error() -> None:
"""Test error for invalid env shell."""
with pytest.raises(ValueError, match="Invalid value for `env.shell`"):
GnuParallelLocalAdapter(
job_name="check_env_shell",
job_dir=Path("."),
yaml_config_path=Path("."),
elements=["sub01"],
env={"kind": "conda", "shell": "fish"},
)
@pytest.mark.parametrize(
"elements, expected_text",
[
@ -55,14 +67,18 @@ def test_GnuParallelLocalAdapter_elements(
@pytest.mark.parametrize(
"pre_run, expected_text",
"pre_run, expected_text, shell",
[
(None, "# Force datalad"),
("# Check this out\n", "# Check this out"),
(None, "# Force datalad", "bash"),
(None, "# Force datalad", "zsh"),
("# Check this out\n", "# Check this out", "bash"),
("# Check this out\n", "# Check this out", "zsh"),
],
)
def test_GnuParallelLocalAdapter_pre_run(
pre_run: Optional[str], expected_text: str
pre_run: Optional[str],
expected_text: str,
shell: str,
) -> None:
"""Test GnuParallelLocalAdapter pre_run().
@ -72,6 +88,8 @@ def test_GnuParallelLocalAdapter_pre_run(
The parametrized pre run text.
expected_text : str
The parametrized expected text.
shell : str
The parametrized expected shell.
"""
adapter = GnuParallelLocalAdapter(
@ -79,21 +97,26 @@ def test_GnuParallelLocalAdapter_pre_run(
job_dir=Path("."),
yaml_config_path=Path("."),
elements=["sub01"],
env={"kind": "conda", "name": "junifer", "shell": shell},
pre_run=pre_run,
)
assert shell in adapter.pre_run()
assert expected_text in adapter.pre_run()
@pytest.mark.parametrize(
"pre_collect, expected_text",
"pre_collect, expected_text, shell",
[
(None, "# This script"),
("# Check this out\n", "# Check this out"),
(None, "# This script", "bash"),
(None, "# This script", "zsh"),
("# Check this out\n", "# Check this out", "bash"),
("# Check this out\n", "# Check this out", "zsh"),
],
)
def test_GnuParallelLocalAdapter_pre_collect(
pre_collect: Optional[str],
expected_text: str,
shell: str,
) -> None:
"""Test GnuParallelLocalAdapter pre_collect().
@ -103,6 +126,8 @@ def test_GnuParallelLocalAdapter_pre_collect(
The parametrized pre collect text.
expected_text : str
The parametrized expected text.
shell : str
The parametrized expected shell.
"""
adapter = GnuParallelLocalAdapter(
@ -110,8 +135,10 @@ def test_GnuParallelLocalAdapter_pre_collect(
job_dir=Path("."),
yaml_config_path=Path("."),
elements=["sub01"],
env={"kind": "venv", "name": "junifer", "shell": shell},
pre_collect=pre_collect,
)
assert shell in adapter.pre_collect()
assert expected_text in adapter.pre_collect()
@ -140,8 +167,10 @@ def test_GnuParallelLocalAdapter_collect() -> None:
@pytest.mark.parametrize(
"env",
[
{"kind": "conda", "name": "junifer"},
{"kind": "venv", "name": "./junifer"},
{"kind": "conda", "name": "junifer", "shell": "bash"},
{"kind": "conda", "name": "junifer", "shell": "zsh"},
{"kind": "venv", "name": "./junifer", "shell": "bash"},
{"kind": "venv", "name": "./junifer", "shell": "zsh"},
],
)
def test_GnuParallelLocalAdapter_prepare(
@ -177,7 +206,7 @@ def test_GnuParallelLocalAdapter_prepare(
adapter.prepare()
assert "GNU parallel" in caplog.text
assert f"Copying run_{env['kind']}" in caplog.text
assert f"Copying run_{env['kind']}.{env['shell']}" in caplog.text
assert "Writing pre_run.sh" in caplog.text
assert "Writing run_test_prepare.sh" in caplog.text
assert "Writing pre_collect.sh" in caplog.text

View file

@ -12,11 +12,11 @@ import pytest
from junifer.api.queue_context import HTCondorAdapter
def test_HTCondorAdapter_env_error() -> None:
def test_HTCondorAdapter_env_kind_error() -> None:
"""Test error for invalid env kind."""
with pytest.raises(ValueError, match="Invalid value for `env.kind`"):
HTCondorAdapter(
job_name="check_env",
job_name="check_env_kind",
job_dir=Path("."),
yaml_config_path=Path("."),
elements=["sub01"],
@ -24,6 +24,18 @@ def test_HTCondorAdapter_env_error() -> None:
)
def test_HTCondorAdapter_env_shell_error() -> None:
"""Test error for invalid env shell."""
with pytest.raises(ValueError, match="Invalid value for `env.shell`"):
HTCondorAdapter(
job_name="check_env_shell",
job_dir=Path("."),
yaml_config_path=Path("."),
elements=["sub01"],
env={"kind": "conda", "shell": "fish"},
)
def test_HTCondorAdapter_collect_error() -> None:
"""Test error for invalid collect option."""
with pytest.raises(ValueError, match="Invalid value for `collect`"):
@ -37,14 +49,18 @@ def test_HTCondorAdapter_collect_error() -> None:
@pytest.mark.parametrize(
"pre_run, expected_text",
"pre_run, expected_text, shell",
[
(None, "# Force datalad"),
("# Check this out\n", "# Check this out"),
(None, "# Force datalad", "bash"),
(None, "# Force datalad", "zsh"),
("# Check this out\n", "# Check this out", "bash"),
("# Check this out\n", "# Check this out", "zsh"),
],
)
def test_HTCondorAdapter_pre_run(
pre_run: Optional[str], expected_text: str
pre_run: Optional[str],
expected_text: str,
shell: str,
) -> None:
"""Test HTCondorAdapter pre_run().
@ -54,6 +70,8 @@ def test_HTCondorAdapter_pre_run(
The parametrized pre run text.
expected_text : str
The parametrized expected text.
shell : str
The parametrized expected shell.
"""
adapter = HTCondorAdapter(
@ -61,22 +79,31 @@ def test_HTCondorAdapter_pre_run(
job_dir=Path("."),
yaml_config_path=Path("."),
elements=["sub01"],
env={"kind": "conda", "name": "junifer", "shell": shell},
pre_run=pre_run,
)
assert shell in adapter.pre_run()
assert expected_text in adapter.pre_run()
@pytest.mark.parametrize(
"pre_collect, expected_text, collect",
"pre_collect, expected_text, collect, shell",
[
(None, "exit 1", "yes"),
(None, "# This script", "on_success_only"),
("# Check this out\n", "# Check this out", "yes"),
("# Check this out\n", "# Check this out", "on_success_only"),
(None, "exit 1", "yes", "bash"),
(None, "exit 1", "yes", "zsh"),
(None, "# This script", "on_success_only", "bash"),
(None, "# This script", "on_success_only", "zsh"),
("# Check this out\n", "# Check this out", "yes", "bash"),
("# Check this out\n", "# Check this out", "yes", "zsh"),
("# Check this out\n", "# Check this out", "on_success_only", "bash"),
("# Check this out\n", "# Check this out", "on_success_only", "zsh"),
],
)
def test_HTCondorAdapter_pre_collect(
pre_collect: Optional[str], expected_text: str, collect: str
pre_collect: Optional[str],
expected_text: str,
collect: str,
shell: str,
) -> None:
"""Test HTCondorAdapter pre_collect().
@ -88,6 +115,8 @@ def test_HTCondorAdapter_pre_collect(
The parametrized expected text.
collect : str
The parametrized collect parameter.
shell : str
The parametrized expected shell.
"""
adapter = HTCondorAdapter(
@ -95,9 +124,11 @@ def test_HTCondorAdapter_pre_collect(
job_dir=Path("."),
yaml_config_path=Path("."),
elements=["sub01"],
env={"kind": "venv", "name": "junifer", "shell": shell},
pre_collect=pre_collect,
collect=collect,
)
assert shell in adapter.pre_collect()
assert expected_text in adapter.pre_collect()
@ -177,8 +208,10 @@ def test_HTCondor_dag(
@pytest.mark.parametrize(
"env",
[
{"kind": "conda", "name": "junifer"},
{"kind": "venv", "name": "./junifer"},
{"kind": "conda", "name": "junifer", "shell": "bash"},
{"kind": "conda", "name": "junifer", "shell": "zsh"},
{"kind": "venv", "name": "./junifer", "shell": "bash"},
{"kind": "venv", "name": "./junifer", "shell": "zsh"},
],
)
def test_HTCondorAdapter_prepare(
@ -215,7 +248,7 @@ def test_HTCondorAdapter_prepare(
assert "Creating HTCondor job" in caplog.text
assert "Creating logs directory" in caplog.text
assert f"Copying run_{env['kind']}" in caplog.text
assert f"Copying run_{env['kind']}.{env['shell']}" in caplog.text
assert "Writing pre_run.sh" in caplog.text
assert "Writing run_test_prepare.submit" in caplog.text
assert "Writing pre_collect.sh" in caplog.text

View file

@ -1,4 +1,4 @@
#!/bin/bash
#!/usr/bin/env bash
if [ $# -lt 2 ]; then
echo "This script is meant to run a command within a conda environment."

View file

@ -0,0 +1,23 @@
#!/usr/bin/env zsh
LeSasse commented 2024-03-20 13:58:58 +00:00 (Migrated from github.com)

would it be viable to have the res as a plain txt containing a text with variable placeholders that get replaced by fill values for a specific shell or are the scripts for the different shell vastly different? just wondering if its desirable avoiding 1 script per shell.

would it be viable to have the `res` as a plain `txt` containing a text with variable placeholders that get replaced by fill values for a specific shell or are the scripts for the different shell vastly different? just wondering if its desirable avoiding 1 script per shell.
synchon commented 2024-03-20 14:03:38 +00:00 (Migrated from github.com)

We would need the file to be created on-demand for that and not have it as a file distributed with the package. I wanted a single file but for simplicity made it separate.

We would need the file to be created on-demand for that and not have it as a file distributed with the package. I wanted a single file but for simplicity made it separate.
if [ $# -lt 2 ]; then
echo "This script is meant to run a command within a conda environment."
echo "It needs at least 2 parameters."
echo "The first one must be the environment name."
echo "The rest will be the command."
exit 255
fi
eval "$(conda shell.zsh hook)"
env_name=$1
echo "Activating ${env_name}"
conda activate "${env_name}"
shift 1
if [ -f "pre_run.sh" ]; then
echo "Sourcing pre_run.sh"
. ./pre_run.sh
fi
echo "Running ${*} in conda environment"
"$@"

22
junifer/api/res/run_venv.bash Executable file
View file

@ -0,0 +1,22 @@
#!/usr/bin/env bash
if [ $# -lt 2 ]; then
echo "This script is meant to run a command within a Python virtual environment."
echo "It needs at least 2 parameters."
echo "The first one must be the virtualenv path."
echo "The rest will be the command."
exit 255
fi
env_path=$1
echo "Activating ${env_path}"
source "${env_path}"/bin/activate
shift 1
if [ -f "pre_run.sh" ]; then
echo "Sourcing pre_run.sh"
. ./pre_run.sh
fi
echo "Running ${*} in Python virtual environment"
"$@"

View file

@ -1,4 +1,4 @@
#!/bin/bash
#!/usr/bin/env zsh
LeSasse commented 2024-03-20 13:59:35 +00:00 (Migrated from github.com)

nice

nice
if [ $# -lt 2 ]; then
echo "This script is meant to run a command within a Python virtual environment."

View file

@ -32,9 +32,7 @@ class JuselessDataladAOMICID1000VBM(PatternDataladDataGrabber):
replacements = ["subject"]
patterns = {
"VBM_GM": {
"pattern": (
"{subject}/mri/mwp1{subject}_run-2_T1w.nii.gz"
),
"pattern": ("{subject}/mri/mwp1{subject}_run-2_T1w.nii.gz"),
"space": "IXI549Space",
},
}

View file

@ -44,9 +44,7 @@ class JuselessDataladIXIVBM(PatternDataladDataGrabber):
replacements = ["site", "subject"]
patterns = {
"VBM_GM": {
"pattern": (
"{site}/{subject}/mri/m0wp1{subject}.nii.gz"
),
"pattern": ("{site}/{subject}/mri/m0wp1{subject}.nii.gz"),
"space": "IXI549Space",
},
}

View file

@ -33,8 +33,7 @@ def test_DataladAOMICID1000() -> None:
assert "BOLD" in out
assert (
out["BOLD"]["path"].name
== f"{test_element}_task-moviewatching_"
out["BOLD"]["path"].name == f"{test_element}_task-moviewatching_"
"space-MNI152NLin2009cAsym_desc-preproc_bold.nii.gz"
)