Skip to content

Commit

Permalink
Fix formatting for cmd operations (#666)
Browse files Browse the repository at this point in the history
* Fix formatting for cmd operations

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* More tests

* Update changelog

* Update flow/project.py

* refactor: Simplify FlowCmdOperation.__call__ logic

* refactor: Revert changes to with_job decorator.

* fix: return correct value with Cmd callable

* refactor: Remove warning filter.

* doc: Improve comments regarding formatting FlowCmdOperations

Co-authored-by: Bradley Dice <bdice@bradleydice.com>

* refactor: Provide conclusive test if cmd operation is formatted

* refactor: Only warn in one place for formatted cmd arguments

* refactor: Remove error on keyword arguments in cmd operations

* refactor: Improve deprecation warning message.

Adds what a user should do instead.

* test: Refactor out common contextmanagers for project execution

* test: Ignore warnings from testing cmd operation formatting

* test: Move assert outside context manager

Co-authored-by: Bradley Dice <bdice@bradleydice.com>

* test: setup_project_subprocess_execution does not discard stderr

At least by default

* doc: Add comment about future removal of cmd op inspect logic

* ci: Update CodeCov CircleCI orb

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
Co-authored-by: Brandon Butler <butlerbr@umich.edu>
  • Loading branch information
5 people authored Oct 13, 2022
1 parent 73f8f5a commit 9217d29
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 112 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
version: 2.1

orbs:
codecov: codecov/codecov@3.2.2
codecov: codecov/codecov@3.2.3

references:
restore_keys: &restore_keys
Expand Down
10 changes: 6 additions & 4 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,23 @@ Changelog
The **signac-flow** package follows `semantic versioning <https://semver.org/>`_.
The numbers in brackets denote the related GitHub issue and/or pull request.

next
====
Version 0.22
============

next -- 2022-xx-xx
------------------
[0.22.0] -- 2022-xx-xx
----------------------

Added
+++++

- Support for formatting with operation function arguments for ``FlowCmdOperation`` (#666, #630).
- The CLI ``status`` command can show document parameters by using flag ``-p doc.PARAM`` (#667).
- ``FlowProject.operation`` now has ``cmd``, ``with_job``, and ``directives`` keyword only arguments (#679, #655, #669).

Changed
+++++++

- Deprecated formatting the output of a ``FlowCmdOperation`` (#666, #630).
- ``@flow.cmd`` and ``flow.with_job`` are deprecated (#679, #669, #665).
- ``@FlowProject.operation.with_directives`` is deprecated (#679, #665).

Expand Down
49 changes: 43 additions & 6 deletions flow/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ class FlowCmdOperation(BaseFlowOperation):
----------
cmd : str or callable
The command to execute the operation. Callable values will be
provided one or more positional arguments (``*jobs``) that are
provided one or more positional arguments that are
instances of :class:`~signac.contrib.job.Job`. String values will be
formatted with ``cmd.format(jobs=jobs)`` where ``jobs`` is a tuple of
:class:`~signac.contrib.job.Job`, or ``cmd.format(jobs=jobs,
Expand All @@ -580,10 +580,47 @@ def __str__(self):
def __call__(self, *jobs):
"""Return the command formatted with the supplied job(s)."""
cmd = self._cmd(*jobs) if callable(self._cmd) else self._cmd
format_arguments = {"jobs": jobs}
if len(jobs) == 1:
format_arguments["job"] = jobs[0]
return cmd.format(**format_arguments)
# The logic below will be removed after version 0.23.0. This is only to temporary fix an
# issue in supporting the formatting of cmd operation in the interim.
format_arguments = {}
if not callable(self._cmd):
format_arguments["jobs"] = jobs
if len(jobs) == 1:
format_arguments["job"] = jobs[0]
formatted_cmd = cmd.format(**format_arguments)
else:
argspec = inspect.getfullargspec(self._cmd)
signature = inspect.signature(self._cmd)
args = {
k: v for k, v in signature.parameters.items() if k != argspec.varargs
}

# get all named positional/keyword arguments with individual names.
for i, arg_name in enumerate(args):
try:
format_arguments[arg_name] = jobs[i]
except IndexError:
format_arguments[arg_name] = args[arg_name].default

# capture any remaining variable positional arguments.
if argspec.varargs:
format_arguments[argspec.varargs] = jobs[len(args) :]

# Capture old behavior which assumes job or jobs in the format string. We need to test
# the truthiness of the key versus the inclusion because in the case of with_job the
# above logic results in format_arguments["jobs"] = ().
if not any(format_arguments.get(key, False) for key in ("jobs", "job")):
if match := re.search("{.*(jobs?).*}", cmd):
# Saves in key jobs or job based on regex match.
format_arguments[match.group(1)] = jobs
formatted_cmd = cmd.format(**format_arguments)
if formatted_cmd != cmd:
warnings.warn(
"Returning format strings in a cmd operation is deprecated as of version 0.22.0 "
"and will be removed in 0.23.0. Users should format the command string.",
FutureWarning,
)
return formatted_cmd


class FlowOperation(BaseFlowOperation):
Expand Down Expand Up @@ -4862,7 +4899,7 @@ def _main_exec(self, args):
if isinstance(operation, FlowCmdOperation):

def operation_function(job):
cmd = operation(job).format(job=job)
cmd = operation(job)
subprocess.run(cmd, shell=True, check=True)

else:
Expand Down
5 changes: 3 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,6 @@ omit =
[bumpversion:file:.zenodo.json]

[tool:pytest]
filterwarnings =
ignore:.*The env argument is deprecated*:DeprecationWarning
filterwarnings =
ignore:.*The env argument is deprecated*:DeprecationWarning
ignore:Returning format strings in a cmd:FutureWarning
Loading

0 comments on commit 9217d29

Please sign in to comment.