Skip to content

Commit

Permalink
Deprecate FlowGroupEntry.with_directives (#696)
Browse files Browse the repository at this point in the history
* refactor: Add directives keyword argument to FlowGroupEntry.__call__

Also deprecates FlowGroupEntry.with_directives.

* test: Update tests to not raise warnings

* doc: Update changelog for #696.

* doc: Make FlowGroupEntry directives change documentation user friendly

* refactor: Use more clear spacing

Co-authored-by: Carl Simon Adorf <[email protected]>

* Require func to be positional-only.

* Fix filterwarnings.

* Update changelog.txt

* Update changelog.txt

* Require func to be positional-only.

Co-authored-by: Carl Simon Adorf <[email protected]>
Co-authored-by: Bradley Dice <[email protected]>
  • Loading branch information
3 people authored Dec 5, 2022
1 parent 38a7b8b commit cdeaf3f
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 14 deletions.
6 changes: 6 additions & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,19 @@ Version 0.23
[0.23.0] -- 20xx-xx-xx
----------------------

Added
+++++

- The ``FlowGroupEntry`` class can be called with a ``directives`` keyword argument: ``FlowGroupEntry(directives={...})`` (#696).

Changed
+++++++

- Deprecated placing ``@FlowProject.pre`` and ``@FlowProject.post`` before the ``FlowProject.operation`` decorator (#690).
- Require ``signac`` version 1.8.0 (#693).
- Deprecated ``alias`` CLI argument to ``flow init`` (#693).
- Algorithm for computing cluster job ids (#695).
- 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):
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"):
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.post.true("test3_true")
@_TestProject.post.false("test3_false")
@_TestProject.post.not_(lambda job: job.doc.test3_false)
Expand Down
9 changes: 6 additions & 3 deletions tests/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -1711,6 +1711,9 @@ class A(FlowProject):
def foo_operation(job):
pass

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

Expand All @@ -1733,12 +1736,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 @@ -1764,7 +1767,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

0 comments on commit cdeaf3f

Please sign in to comment.