Skip to content

Commit

Permalink
Enhance run() placeholder substitutions to honor configuration defa…
Browse files Browse the repository at this point in the history
…ults

Previously, `run()` would not recognize configuration defaults for
placeholder substitution. This means that any placeholders globally
declared in `datalad.interface.common_cfg`, or via `register_config()`
in DataLad extensions would not be effective.

This changeset makes run's `format_command()` helper include such
defaults explicitly, and thereby enable the global declaration of
substitution defaults.

Moreoever a `{python}` placeholder is now defined via this mechanism,
and points to the value of `sys.executable` by default. This particular
placeholder was found to be valueable for improving the portability of
run-recording across (specific) Python versions, or across different
(virtual) environments. See
datalad/datalad-container#224 for an example
use case.

A corresponding test is included.

The ability to keep run-records paramterizable, in particular, for
interpreters can also aid longevity (think platform-specific choices
to call a system Python executable `python3` for some years, and then
switch back.

Related datalad/datalad-container#250
  • Loading branch information
mih committed Oct 13, 2023
1 parent a96c51c commit 02c930a
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
8 changes: 7 additions & 1 deletion datalad/core/local/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
build_doc,
eval_results,
)
from datalad.interface.common_cfg import definitions as cfg_defs
from datalad.interface.common_opts import (
jobs_opt,
save_message_opt,
Expand Down Expand Up @@ -623,7 +624,12 @@ def format_command(dset, command, **kwds):
command = normalize_command(command)
sfmt = SequenceFormatter()

for k, v in dset.config.items("datalad.run.substitutions"):
for k in set(cfg_defs.keys()).union(dset.config.keys()):
v = dset.config.get(
k,
# pull a default from the config definitions
# if we have no value, but a key
cfg_defs.get(k, {}).get('default', None))
sub_key = k.replace("datalad.run.substitutions.", "")
if sub_key not in kwds:
kwds[sub_key] = v
Expand Down
20 changes: 20 additions & 0 deletions datalad/core/local/tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
mkdir,
remove,
)
import pytest
from unittest.mock import patch

from datalad.api import (
Expand All @@ -43,6 +44,7 @@
from datalad.tests.utils_pytest import (
DEFAULT_BRANCH,
OBSCURE_FILENAME,
SkipTest,
assert_false,
assert_in,
assert_in_results,
Expand Down Expand Up @@ -781,3 +783,21 @@ class dset:
eq_(_format_iospecs(['{dummy}'],
**_get_substitutions(dset)),
['a', 'b'])


def test_substitution_config_default(tmp_path):
ds = Dataset(tmp_path).create(result_renderer='disabled')

if ds.config.get('datalad.run.substitutions.python') is not None:
# we want to test default handling when no config is set
raise SkipTest(
'Test assumptions conflict with effective configuration')

# the {python} placeholder is not explicitly defined, but it has
# a default, which run() should discover and use
res = ds.run('{python} -c "True"', result_renderer='disabled')
assert_result_count(res, 1, action='run', status='ok')

# make sure we could actually detect breakage with the check above
with pytest.raises(IncompleteResultsError):
ds.run('{python} -c "breakage"', result_renderer='disabled')
8 changes: 8 additions & 0 deletions datalad/interface/common_cfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from os import environ
from os.path import expanduser
from os.path import join as opj
import sys
import time

from platformdirs import AppDirs
Expand Down Expand Up @@ -546,6 +547,13 @@ def get_default_ssh():
'type': EnsureInt(),
'default': 8,
},
'datalad.run.substitutions.python': {
'ui': ('question', {
'title': 'Substitution for {python} placeholder',
'text': 'Path to a Python interpreter executable'}),
'type': EnsureStr(),
'default': sys.executable,
},
'datalad.runtime.max-annex-jobs': {
'ui': ('question', {
'title': 'Maximum number of git-annex jobs to request when "jobs" option set to "auto" (default)',
Expand Down

0 comments on commit 02c930a

Please sign in to comment.