Skip to content

Commit

Permalink
Return parameter values from the orderly.parameters call.
Browse files Browse the repository at this point in the history
Previously we would inject parameter values as global variables before
the script starts executing. While this follows the behaviour of
orderly2, it confuses a lot of Python tooling, including linters and
type checkers. It also did not support executing orderly scripts outside
of a packet context (eg. interactively).

The `orderly.parameters` function now returns the parameter values as a
dictionary, which we can get from the global context. When running
outside of a packet context, it returns the parameters' default values.
  • Loading branch information
plietar committed May 3, 2024
1 parent c42fca9 commit 927fafa
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 34 deletions.
19 changes: 16 additions & 3 deletions src/orderly/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from outpack import util
from outpack.copy_files import copy_files
from outpack.search import search_unique
from outpack.util import pl


@dataclass_json()
Expand All @@ -31,7 +32,7 @@ def empty():
return Description(None, None, None)


def parameters(**kwargs): # noqa: ARG001
def parameters(**kwargs):
"""Declare parameters used in a report.
Parameters
Expand All @@ -42,9 +43,21 @@ def parameters(**kwargs): # noqa: ARG001
Returns
-------
Nothing, this function has no effect at all!
The parameters that were passed to the report. If running outside of an
orderly context, this returns kwargs unmodified.
"""
pass
ctx = get_active_context()
if ctx.is_active:
# We don't need to apply defaults from kwargs as the packet runner
# already did so.
return ctx.parameters
else:
missing = [k for k, v in kwargs.items() if v is None]
if missing:
msg = f"No value was specified for {pl(missing, 'parameter')} {', '.join(missing)}."
raise Exception(msg)

return kwargs


def resource(files):
Expand Down
12 changes: 4 additions & 8 deletions src/orderly/current.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ def __init__(self):

@dataclass
class OrderlyContext:
# Indicates if a packet is currently running
is_active: bool
# The active packet, if one is running
packet: Optional[Packet]
# the path to the packet running directory. This is a pathlib Path object
Expand All @@ -32,8 +30,6 @@ class OrderlyContext:
parameters: dict
# The name of the packet
name: str
# The id of the packet, only non-None if active
id: Optional[str]
# Special orderly custom metadata
orderly: OrderlyCustomMetadata
# Options used when searching for dependencies
Expand All @@ -42,14 +38,12 @@ class OrderlyContext:
@staticmethod
def from_packet(packet, path_src, search_options=None):
return OrderlyContext(
is_active=True,
packet=packet,
path=packet.path,
path_src=Path(path_src),
root=packet.root,
parameters=packet.parameters,
name=packet.name,
id=packet.id,
orderly=OrderlyCustomMetadata(),
search_options=search_options,
)
Expand All @@ -58,19 +52,21 @@ def from_packet(packet, path_src, search_options=None):
def interactive():
path = Path(os.getcwd())
return OrderlyContext(
is_active=False,
packet=None,
path=path,
path_src=path,
root=detect_orderly_interactive_root(path),
parameters={},
name=path.name,
id=None,
orderly=OrderlyCustomMetadata(),
# TODO: expose a way of configuring this
search_options=None,
)

@property
def is_active(self):
return self.packet is not None


class ActiveOrderlyContext:
_context = None
Expand Down
20 changes: 10 additions & 10 deletions src/orderly/read.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,21 @@ def read_body(self, stmts):
self._read_stmt(stmt)

def _read_stmt(self, stmt):
if (name := _match_orderly_call(stmt)) is not None:
if name == "parameters":
self._read_parameters(stmt.value)

if isinstance(stmt, (ast.Expr, ast.Assign)):
self._read_expr(stmt.value)
elif _match_name_check(stmt) == "__main__":
self.read_body(stmt.body)

def _read_expr(self, expr):
name = _match_orderly_call(expr)
if name == "parameters":
self._read_parameters(expr)

def _read_parameters(self, call):
if call.args:
msg = "All arguments to 'parameters()' must be named"
raise Exception(msg)

data = {}
for kw in call.keywords:
nm = kw.arg
Expand All @@ -54,7 +58,7 @@ def _read_parameters(self, call):
if not _is_valid_parameter_value(value):
msg = f"Invalid value for argument '{nm}' to 'parameters()'"
raise Exception(msg)
data[nm] = kw.value.value
data[nm] = value.value

if self.parameters is not None:
msg = f"Duplicate call to 'parameters()' on line {call.lineno}"
Expand Down Expand Up @@ -90,11 +94,7 @@ def _match_name_check(stmt):
return None


def _match_orderly_call(stmt):
if not isinstance(stmt, ast.Expr):
return None

expr = stmt.value
def _match_orderly_call(expr):
if not isinstance(expr, ast.Call):
return None

Expand Down
5 changes: 2 additions & 3 deletions src/orderly/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def _packet_builder(

try:
orderly = _run_report_script(
packet, path, path_src, entrypoint, parameters, search_options
packet, path, path_src, entrypoint, search_options
)
_check_artefacts(orderly, path)
except:
Expand All @@ -84,15 +84,14 @@ def _packet_builder(


def _run_report_script(
packet, path, path_src, entrypoint, parameters, search_options
packet, path, path_src, entrypoint, search_options
) -> OrderlyCustomMetadata:
try:
with ActiveOrderlyContext(packet, path_src, search_options) as orderly:
# Calling runpy with the full path to the script gives us better
# tracebacks
runpy.run_path(
str(path / entrypoint),
init_globals=parameters,
run_name="__main__",
)

Expand Down
7 changes: 2 additions & 5 deletions tests/orderly/examples/mp/mp.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,16 @@

import orderly

# TODO(mrc-5255)
# ruff: noqa: F821


def square(x):
return x * x


if __name__ == "__main__":
orderly.parameters(method=None)
params = orderly.parameters(method=None)
orderly.artefact("Squared numbers", "result.txt")

mp = multiprocessing.get_context(method) # type: ignore
mp = multiprocessing.get_context(params["method"])
data = [random.random() for _ in range(10)]

with mp.Pool(5) as p:
Expand Down
7 changes: 2 additions & 5 deletions tests/orderly/examples/parameters/parameters.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import orderly

# TODO(mrc-5255): We'll rethink this strategy soon
# ruff: noqa: F821

orderly.parameters(a=1, b=None)
parameters = orderly.parameters(a=1, b=None)
with open("result.txt", "w") as f:
f.write(f"a: {a}\nb: {b}\n") # type: ignore
f.write(f"a: {parameters['a']}\nb: {parameters['b']}\n")
5 changes: 5 additions & 0 deletions tests/orderly/test_read.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ def test_read_simple_trivial_parameters():
assert ab == {"parameters": {"a": None, "b": 1}}


def test_read_parameters_assignment():
ab = _read_py("params = orderly.parameters(a=None, b=1)")
assert ab == {"parameters": {"a": None, "b": 1}}


def test_skip_over_uninteresting_code():
code = """
def foo():
Expand Down
32 changes: 32 additions & 0 deletions tests/orderly/test_run_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,35 @@ def test_can_use_dependency_without_packet(tmp_path):
assert len(result.files) == 1
assert result.files["input.txt"].path == "result.txt"
assert src.joinpath("input.txt").exists()


def test_can_use_parameters(tmp_path):
root = helpers.create_temporary_root(tmp_path)
src = tmp_path / "src" / "x"
src.mkdir(parents=True)

p = Packet(root, src, "tmp", parameters={"x": 1, "y": "foo"})
with ActiveOrderlyContext(p, src):
with transient_working_directory(src):
params = orderly.parameters(x=None, y=None)
assert params == {"x": 1, "y": "foo"}


def test_can_use_parameters_without_packet(tmp_path):
helpers.create_temporary_root(tmp_path)
src = tmp_path / "src" / "x"
src.mkdir(parents=True)

with transient_working_directory(src):
p = orderly.parameters(x=1, y="foo")
assert p == {"x": 1, "y": "foo"}

with pytest.raises(
Exception, match="No value was specified for parameter x."
):
orderly.parameters(x=None, y="foo")

with pytest.raises(
Exception, match="No value was specified for parameters x, y."
):
orderly.parameters(x=None, y=None)

0 comments on commit 927fafa

Please sign in to comment.