Expressions, Parameters and Renderers #55

philtweir · 2024-08-25T00:14:45Z

✨ Feature

Description

Please describe and motivate the new feature you've implemented.

Changes Made

Workflows

Arbitrarily nested fields
Support for sympy expressions
All references as sympy Symbols
Surfacing deeply nested references
Standardize on workflow (removes subworkflow and nested_step)
Significant speedups for complex workflows (10-100x)
Better support for schema types (TypedDict, dataclasses and attrs)

Renderers

Renderers can load from a package
Some tests for renderer loading
CWL: support for expressions
CWL: improvements to type support

Additional Information

Still TODO (help appreciated):

update the documentation to match (examples are updated, but not text)
docstrings for tests

Checklist

I have included a detailed description of the feature.
I have provided relevant documentation for my feature.
I have added the necessary tests.

Who can review?

@KamenDimitrov97 @elleryames

…fields

…e supplied

philtweir

Add change notes

.github/workflows/python-test-ci.yml

docs/quickstart.md

src/dewret/__main__.py

philtweir · 2024-08-28T12:25:02Z

src/dewret/__main__.py

    except Exception as exc:
        import traceback

        print(exc, exc.__cause__, exc.__context__)
        traceback.print_exc()
    else:
-        if pretty:
-            yaml.dump(cwl, sys.stdout, indent=2)
+        if len(rendered) == 1:


Change: Write logic for outputting different formats

src/dewret/annotations.py

philtweir · 2024-08-28T12:44:45Z

src/dewret/workflow.py


    @classmethod
-    def assimilate(cls, left: Workflow, right: Workflow) -> "Workflow":
+    def assimilate(cls, *workflow_args: Workflow) -> "Workflow":


Change: improve performance by merging n workflows together simultaneously, instead of pairwise (for example, if we had a function with 5 step results as input arguments, this would get called 4 times, but now gets called once)

philtweir · 2024-08-28T12:51:46Z

src/dewret/workflow.py

@@ -657,14 +765,206 @@ def __workflow__(self) -> Workflow:
        ...


-class Reference:
-    """Superclass for all symbolic references to values."""
+class FieldableProtocol(Protocol):


Change: a mixin for structures that use fields

philtweir · 2024-08-28T12:53:07Z

src/dewret/workflow.py

            but ParameterReferences are specific, this carries the actual workflow reference.

    Returns:
        Workflow that the referee is related to.
    """

-    parameter: Parameter[RawType]
-    __workflow__: Workflow
+    class ParameterReferenceMetadata(Generic[T]):


Change: moving metadata (more) into an attribute called _ for step and parameter references.

philtweir · 2024-08-28T12:54:26Z

src/dewret/workflow.py


-U = TypeVar("U")
+class IterableParameterReference(IterableMixin[U], ParameterReference[U]):


Change: add iterable references to handle things we can loop through or index into

philtweir · 2024-08-28T12:55:25Z

src/dewret/workflow.py

@@ -1179,3 +1633,104 @@ def is_task(task: Lazy) -> bool:
        True if `task` is indeed a task.
    """
    return isinstance(task, LazyEvaluation)
+
+def expr_to_references(expression: Any, remap: Callable[[Any], Any] | None = None) -> tuple[ExprType, list[Reference[Any] | Parameter[Any]]]:


Change: add support for sympy expressions involving references.

KamenDimitrov97

I've left some comments with change suggestions.
I still have a few files to review and test out.

KamenDimitrov97 · 2024-08-29T09:27:53Z

src/dewret/render.py

+        render_module = cast(BaseRenderModule, module)
+    else:
+        render_module = renderer
+    if not isinstance(render_module, StructuredRenderModule):


This looks a bit hard to follow.

This is understandable

# This is understandable render_module: BaseRenderModule if isinstance(renderer, Path): if (render_dir := str(renderer.parent)) not in sys.path: sys.path.append(render_dir) # Attempt to load renderer as package, falling back to a single module otherwise. # This enables relative imports in renderers and therefore the ability to modularize. module = load_module_or_package("__renderer__", renderer) sys.modules["__renderer_mod__"] = module render_module = cast(BaseRenderModule, module) else: render_module = renderer

But can we handle conditionals here in a similar way to this?
Since the renderer has to be Path | RawRenderModule | StructuredRenderModule and StructuredRenderModule is a BaseRenderModule
and when we render_module = cast(BaseRenderModule, module)

I can see what every modules implementation is without having to think about it.
My first thought was what happened to the BaseRenderModule

if isinstance(render_module, RawRenderModule): return render_module.render_raw elif isinstance(render_module, (StructuredRenderModule)): def _render(workflow: Workflow, render_module: StructuredRenderModule, pretty: bool=False, **kwargs: RenderConfiguration) -> dict[str, str]: rendered = render_module.render(workflow, **kwargs) return { key: structured_to_raw(value, pretty=pretty) for key, value in rendered.items() } return cast(RenderCall, partial(_render, render_module=render_module, pretty=pretty)) raise NotImplementedError("This render module neither seems to be a structured nor a raw render module.")

@KamenDimitrov97 To solve

src/dewret/render.py

.github/workflows/python-test-ci.yml

src/dewret/core.py

KamenDimitrov97 · 2024-08-29T09:45:19Z

src/dewret/core.py

+        metadata += list(parent_metadata)
+    return parent_type, tuple(metadata)
+
+RenderConfiguration = dict[str, RawType]


Possibly irrelevant ignore if it is but maybe a comment like this

# Configuration settings for the renderer

@KamenDimitrov97 keep
Generic type for configuration settings for the renderer

src/dewret/backends/backend_dask.py

KamenDimitrov97 · 2024-08-29T10:16:41Z

src/dewret/annotations.py

+        If `fn` is a class, it takes the constructor, and if it is a method, it takes
+        the `__func__` attribute.
+        """
+        self.fn = (


I don't know how I feel about these.

if inspect.isclass(fn): self.fn = fn.__init__ elif inspect.ismethod(fn): self.fn = fn.__func__ else: self.fn = fn

It might be a lack of years of experience with python but it just seems harder to read

@KamenDimitrov97 change this

KamenDimitrov97 · 2024-08-29T10:18:58Z

docs/renderer_tutorial.md

 from dewret.workflow import Lazy
-from dewret.workflow import Reference, Raw, Workflow, Step, Task
-from dewret.utils import BasicType
+from dewret.workflow import Reference, Workflow, Step, Task

 RawType = typing.Union[BasicType, list[str], list["RawType"], dict[str, "RawType"]]


Follow up comment: We should also update the documentations and tutorials to use | instead of 'union'

docs/renderer_tutorial.md

src/dewret/__main__.py

KamenDimitrov97 · 2024-08-30T11:02:36Z

src/dewret/__main__.py

+        if len(rendered) == 1:
+            with opener("", "w") as output_f:
+                output_f.write(rendered["__root__"])
+        elif "%" in output:


keep this in outside the utility function

src/dewret/__main__.py

… added explanitory comments

…s as a package nor isolated module

philtweir · 2024-09-09T21:22:15Z

tests/test_render_module.py

+    assert entry.relline == 2
+    assert "attempted relative import with no known parent package" in str(exc.value)
+
+    nonfrender_py = Path(__file__).parent / "_lib/nonfrender.py"


Testing module runtime check against protocol.

…s as a package nor isolated module

… to cause errors

…ded for a custom renderer to be considered a dewret renderer

philtweir added 30 commits August 21, 2024 18:09

wip(renderers): can pick renderers dynamically

2e7e6e7

fix(renderers): make the renderer semantics less janky

aad87a5

feat(renderers): add the ability to pass renderer arguments via CLI/YAML

c58c17e

feat(renderers): add way to output subworkflows to files

d792f76

feat: support list/tuple return values

b2c10e2

feat(annotations): add ability to annotate variables

f9e6c9b

fix: correct tests

d5b8c65

wip: add configuration

470de20

fix: scope thread pools to a single render call and add context copying

be9b862

fix: make sure the globals are updated when a subworkflow is running

7c34dbe

fix: tests

c6dc355

fix: integrate flatten_all_nested configuration

a7c1fe9

fix: correctly handle flattened nested tasks

dee6815

fix: confirm tuples can be returned

363be81

fix: step reference works as tuple

7aeccd9

fix: tidy up type parsing

03c8c8b

fix: nested raw is parseable

b50b150

fix(annotations): correct name to AtRender

109dbf9

feat(sympy): handle arithmetic/structure manipulation of references

6cb2ca1

feat(sympy): more flexible parameter support

0f846e7

fix: fix render load order

ff81ad4

feat(iterated): allow spreading

e3b9b28

feat(fields): make it possible to use a plain dict if configured for …

38825fa

…fields

fix: is_raw_type should handle union types

0f047fb

feat(positional): allow positional arguments if configuration overrid…

9468595

…e supplied

fix(sympy): support none in dicts

333967f

fix(sympy): support none in dicts

079c00b

fix: expressions should consider Raw as a raw type

213c514

fix: sorted should not break for two identical steps

08db69f

fix: efficiency improvements

3dd35d1

fix: do not lose the field structure when iterating

d890bd3

philtweir commented Aug 28, 2024

View reviewed changes

fix: add a catch for a bug in the first import

351d344

KamenDimitrov97 reviewed Aug 29, 2024

View reviewed changes

fix: remove unnecessary abstractmethod annotation

3bd55e4

KamenDimitrov97 reviewed Aug 30, 2024

View reviewed changes

philtweir and others added 9 commits August 30, 2024 12:04

fix: remove unnecessary abstractmethod annotation

a7c0d8d

chore: Replace all Unions with | for consistency

c32469e

chore: Refactored render.py:get_render_method for better readability,…

f6f478b

… added explanitory comments

chore: Formated methods

e76a0a9

docs: Added docstrings to annotation tests

a0655c0

docs: Added docstrings for tests

aa7d6e4

fix: Formating

3bff5e5

fix: double-check importing correctly errors if render neither behave…

7ae5a7a

…s as a package nor isolated module

fix: check that importing a non-compliant render module throws an error

2a2d790

philtweir commented Sep 9, 2024

View reviewed changes

philtweir and others added 7 commits September 9, 2024 22:39

fix: double-check importing correctly errors if render neither behave…

241f338

…s as a package nor isolated module

fix: double-check importing correctly errors if render neither behave…

fe5253c

…s as a package nor isolated module

fix: ensure pytest ignores tests._lib

c28a31e

fix: correct name of default_config

713882a

fix: remove Graph import from dask.typing in backend_dask as it seems…

6714786

… to cause errors

ops: enable example tests in CI

dd27801

feat: Added a test for default renderer

b6677c0

KamenDimitrov97 force-pushed the feature/parameter-type-improvements branch from f9b3e8c to b6677c0 Compare September 17, 2024 11:20

KamenDimitrov97 force-pushed the feature/parameter-type-improvements branch from ee37865 to 2fa4145 Compare September 30, 2024 12:13

feat: Added a test for default renderer

3b9ccfe

KamenDimitrov97 force-pushed the feature/parameter-type-improvements branch from 2fa4145 to 3b9ccfe Compare September 30, 2024 12:15

KamenDimitrov97 added 2 commits September 30, 2024 17:49

feat: Abstracted write_rendered_output from the main dewret CLI

f519c47

docs: Updated renderer tutorial with the protocols implementation nee…

a25d3c2

…ded for a custom renderer to be considered a dewret renderer

KamenDimitrov97 merged commit 229102b into release/0.10.0 Sep 30, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expressions, Parameters and Renderers #55

Expressions, Parameters and Renderers #55

philtweir commented Aug 25, 2024

philtweir left a comment

philtweir Aug 28, 2024

philtweir Aug 28, 2024

philtweir Aug 28, 2024

philtweir Aug 28, 2024

philtweir Aug 28, 2024

philtweir Aug 28, 2024

KamenDimitrov97 left a comment

KamenDimitrov97 Aug 29, 2024 •

edited

Loading

KamenDimitrov97 Aug 30, 2024

KamenDimitrov97 Aug 29, 2024

KamenDimitrov97 Aug 30, 2024

KamenDimitrov97 Aug 29, 2024

KamenDimitrov97 Aug 30, 2024

KamenDimitrov97 Aug 29, 2024

KamenDimitrov97 Aug 30, 2024

philtweir Sep 9, 2024


		U = TypeVar("U")
		class IterableParameterReference(IterableMixin[U], ParameterReference[U]):

Expressions, Parameters and Renderers #55

Expressions, Parameters and Renderers #55

Conversation

philtweir commented Aug 25, 2024

✨ Feature

Description

Changes Made

Workflows

Renderers

Additional Information

Checklist

Who can review?

philtweir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KamenDimitrov97 left a comment

Choose a reason for hiding this comment

KamenDimitrov97 Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KamenDimitrov97 Aug 29, 2024 •

edited

Loading