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

Patch run() placeholder substitutions to honor configuration defaults #485

Merged
merged 1 commit into from
Oct 15, 2023
Merged
Show file tree
Hide file tree
Changes from all 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 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 valuable for improving the portability of
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(

Check warning on line 15 in datalad_next/patches/tests/test_run.py

View check run for this annotation

Codecov / codecov/patch

datalad_next/patches/tests/test_run.py#L15

Added line #L15 was not covered by tests
'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
Loading