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

Deprecate FlowGroupEntry.with_directives #696

Merged
merged 14 commits into from
Dec 5, 2022
Merged
6 changes: 6 additions & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,17 @@ Version 0.23

[0.23.0] -- 20xx-xx-xx

Added
+++++

- The ``FlowGroupEntry`` class can be called with a ``directives`` keyword argument now e.g. ``FlowGroupEntry(directives={...})``(#696).
bdice marked this conversation as resolved.
Show resolved Hide resolved

Changed
+++++++

- Require ``signac`` version 1.8.0 (#693).
- Deprecated ``alias`` CLI argument to ``flow init`` (#693).
- Deprecated ``FlowGroupEntry.with_directives`` in favor of a directives keyword argument in ``FlowGroupEntry()``(#696).

Fixed
+++++
Expand Down
46 changes: 36 additions & 10 deletions flow/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -634,12 +634,10 @@ class FlowGroupEntry:

Operation functions can be marked for inclusion into a :class:`FlowGroup`
by decorating the functions with a corresponding :class:`FlowGroupEntry`.
If the operation requires specific directives, :meth:`~.with_directives`
accepts keyword arguments that are mapped to directives and returns a
decorator that can be applied to the operation to mark it for inclusion in
the group and indicate that it should be executed using the specified
directives. This overrides the default directives specified by
:meth:`flow.directives`.
If the operation requires group specific directives, calling the
:class:`FlowGroupEntry` with the keyword argument ``directives`` allows the
setting of directives for the exclusively for the group. Doing this overrides
the default directives specified by :meth:`FlowProject.operation`.

Parameters
----------
Expand Down Expand Up @@ -673,7 +671,7 @@ def __init__(
# `@operation`.
self.group_aggregator = group_aggregator

def __call__(self, func):
def __call__(self, func=None, *, directives=None):
"""Add the function into the group's operations.

This call operator allows the class to be used as a decorator.
Expand All @@ -683,12 +681,23 @@ def __call__(self, func):
func : callable
The function to decorate.

directives : dict
Directives to use for resource requests and execution.
The directives specified in this decorator are only applied when
executing the operation through the :class:`FlowGroup`.
To apply directives to an individual operation executed outside of the
group, see :meth:`.FlowProject.operation`.

Returns
-------
callable
func
The decorated function.

"""
if func is None:
return functools.partial(self._internal_call, directives=directives)
return self._internal_call(func, directives=directives)

def _internal_call(self, func, *, directives):
bdice marked this conversation as resolved.
Show resolved Hide resolved
if not any(
func == op_func for _, op_func in self._project._OPERATION_FUNCTIONS
):
Expand All @@ -702,6 +711,13 @@ def __call__(self, func):
f"Cannot reregister operation '{func}' with the group '{self.name}'."
)
func._flow_groups[self._project].add(self.name)
if directives is None:
return func

if hasattr(func, "_flow_group_operation_directives"):
b-butler marked this conversation as resolved.
Show resolved Hide resolved
func._flow_group_operation_directives[self.name] = directives
else:
func._flow_group_operation_directives = {self.name: directives}
return func

def _set_directives(self, func, directives):
Expand Down Expand Up @@ -729,6 +745,9 @@ def with_directives(self, directives):
To apply directives to an individual operation executed outside of the
group, see :meth:`.FlowProject.operation`.

Note:
This method has been deprecated and will be removed in 0.24.0.

Parameters
----------
directives : dict
Expand All @@ -740,6 +759,13 @@ def with_directives(self, directives):
A decorator which registers the operation with the group using the
specified directives.
"""
_deprecated_warning(
deprecation="@FlowGroupEntry.with_directives",
alternative="Use the directives keyword argument in base decorator e.g. "
"@FlowGroupEntry(directives={...}).",
deprecated_in="0.23.0",
removed_in="0.24.0",
)

def decorator(func):
self._set_directives(func, directives)
Expand All @@ -763,7 +789,7 @@ class FlowGroup:

group = FlowProject.make_group(name='example_group')

@group.with_directives({"nranks": 4})
@group(directives={"nranks": 4})
@FlowProject.operation({"nranks": 2, "executable": "python3"})
def op1(job):
pass
Expand Down
2 changes: 1 addition & 1 deletion tests/define_test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def op2(job):
job.document.test = os.getpid()


@group2.with_directives(dict(omp_num_threads=4))
@group2(directives={"omp_num_threads": 4})
@_TestProject.operation(directives={"ngpu": 1, "omp_num_threads": 1})
@_TestProject.post.true("test3_true")
@_TestProject.post.false("test3_false")
Expand Down
7 changes: 4 additions & 3 deletions tests/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -1703,6 +1703,7 @@ class A(FlowProject):
def foo_operation(job):
pass

@pytest.mark.filterwarnings("ignore:*with_directives*:FutureWarning")
def test_repeat_operation_group_directives_definition(self):
"""Test that operations cannot be registered with group directives multiple times."""

Expand All @@ -1725,12 +1726,12 @@ class A(flow.FlowProject):

group = A.make_group("group")

@group.with_directives(dict(ngpu=2, nranks=4))
@group(directives={"ngpu": 2, "nranks": 4})
@A.operation
def op1(job):
pass

@group.with_directives(dict(ngpu=2, nranks=4))
@group(directives={"ngpu": 2, "nranks": 4})
@A.operation
def op2(job):
pass
Expand All @@ -1756,7 +1757,7 @@ class A(flow.FlowProject):

group = A.make_group("group")

@group.with_directives(dict(ngpu=2, nranks=4))
@group(directives={"ngpu": 2, "nranks": 4})
@A.operation
def op1(job):
pass
Expand Down