Skip to content

Commit

Permalink
Patch run() placeholder substitutions to honor configuration defaults
Browse files Browse the repository at this point in the history
This is the companion of datalad/datalad#7509

Closes #478
  • Loading branch information
mih committed Oct 13, 2023
1 parent c64978b commit 76af25f
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 0 deletions.
1 change: 1 addition & 0 deletions datalad_next/patches/enabled.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@
test_keyring,
customremotes_main,
create_sibling_gitlab,
run,
)
84 changes: 84 additions & 0 deletions datalad_next/patches/run.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
"""Enhance ``run()`` placeholder substitutions to honor configuration defaults
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 patch 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

Check failure on line 13 in datalad_next/patches/run.py

View workflow job for this annotation

GitHub Actions / Check for spelling errors

valueable ==> valuable
run-recording across (specific) Python versions, or across different (virtual)
environments. See https://github.com/datalad/datalad-container/issues/224 for
an example use case.
https://github.com/datalad/datalad/pull/7509
"""

import sys

from datalad.core.local.run import (
GlobbedPaths,
SequenceFormatter,
normalize_command,
quote_cmdlinearg,
)
from datalad.interface.common_cfg import definitions as cfg_defs
from datalad.support.constraints import EnsureStr
from datalad.support.extensions import register_config

from . import apply_patch


# This function is taken from datalad-core@a96c51c0b2794b2a2b4432ec7bd51f260cb91a37
# datalad/core/local/run.py
# The change has been proposed in https://github.com/datalad/datalad/pull/7509
def format_command(dset, command, **kwds):
"""Plug in placeholders in `command`.
Parameters
----------
dset : Dataset
command : str or list
`kwds` is passed to the `format` call. `inputs` and `outputs` are converted
to GlobbedPaths if necessary.
Returns
-------
formatted command (str)
"""
command = normalize_command(command)
sfmt = SequenceFormatter()

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

for name in ["inputs", "outputs"]:
io_val = kwds.pop(name, None)
if not isinstance(io_val, GlobbedPaths):
io_val = GlobbedPaths(io_val, pwd=kwds.get("pwd"))
kwds[name] = list(map(quote_cmdlinearg, io_val.expand(dot=False)))
return sfmt.format(command, **kwds)


apply_patch(
'datalad.core.local.run', None, 'format_command', format_command)
register_config(
'datalad.run.substitutions.python',
'Substitution for {python} placeholder',
description='Path to a Python interpreter executable',
type=EnsureStr(),
default=sys.executable,
dialog='question',
)
25 changes: 25 additions & 0 deletions datalad_next/patches/tests/test_run.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import pytest

from datalad_next.exceptions import IncompleteResultsError
from datalad_next.tests.utils import (
SkipTest,
assert_result_count,
)


def test_substitution_config_default(existing_dataset):
ds = existing_dataset

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')
1 change: 1 addition & 0 deletions docs/source/patches.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ DataLad patches
push_to_export_remote
test_keyring
siblings
run

0 comments on commit 76af25f

Please sign in to comment.