Skip to content

Pwd converter#175

Merged
liamhuber merged 10 commits intomainfrom
pwd-converter
Mar 7, 2026
Merged

Pwd converter#175
liamhuber merged 10 commits intomainfrom
pwd-converter

Conversation

@liamhuber
Copy link
Member

@liamhuber liamhuber commented Mar 6, 2026

PR introduces bidirectional conversion between flowrep and python-workflow-definition (PWD) recipe formats.

In the flowrep->PWD direction there are obviously constraints, namely that you can only convert flat, all-atomic workflows. In the PWD->flowrep direction there are constraints, but I think they're mostly the result of PWD not validating very strictly -- e.g. if you write a cyclic recipe you can get it validated in PWD but flowrep will throw an exception if you try to convert it. IMO that's an implementation restriction, not a conceptual one.

Conceptually, the conversions are not totally lossless. In particular, round tripping does not guarantee that you'll get back the same node names/ids (which are semi-arbitrary). Also, PWD doesn't differentiate the module vs qualname aspect of function references, so we're left guessing at how to split it1, and no version information is held whatsoever so we need to leave that blank.

But barring these losses, all three of the PWD test cases and a representative sample of flowrep recipes roundtrip quite nicely.

The new converters are exposed in the API, but not promoted all the way to be directly available on the module. A simple example:

from flowrep.models import workflow
from flowrep.models.api import converters


def foo(x=0, y=0):
    return x + y

def bar(a, b):
    return a - b

@workflow
def some_flat_wf(x, y, a):
    v = foo(x, y)
    w = bar(a, v)
    return w

pwd = converters.flowrep2pwd(some_flat_wf.flowrep_recipe, x=1, y=2, a=3)
print(pwd.model_dump_json(indent=2))

Gives

{
  "version": "0.0.1",
  "nodes": [
    {
      "id": 0,
      "type": "input",
      "name": "x",
      "value": 1
    },
    {
      "id": 1,
      "type": "input",
      "name": "y",
      "value": 2
    },
    {
      "id": 2,
      "type": "input",
      "name": "a",
      "value": 3
    },
    {
      "id": 3,
      "type": "output",
      "name": "w"
    },
    {
      "id": 4,
      "type": "function",
      "value": "__main__.foo"
    },
    {
      "id": 5,
      "type": "function",
      "value": "__main__.bar"
    }
  ],
  "edges": [
    {
      "target": 4,
      "targetPort": "x",
      "source": 0,
      "sourcePort": null
    },
    {
      "target": 4,
      "targetPort": "y",
      "source": 1,
      "sourcePort": null
    },
    {
      "target": 5,
      "targetPort": "a",
      "source": 2,
      "sourcePort": null
    },
    {
      "target": 5,
      "targetPort": "b",
      "source": 4,
      "sourcePort": "output_0"
    },
    {
      "target": 3,
      "targetPort": null,
      "source": 5,
      "sourcePort": "output_0"
    }
  ]
}

(The 0.0.1 version is an artefact of the fact I'm running git clone repo source code here...)

Then we can get back to our original flowrep representation:

fr, defaults = converters.pwd2flowrep(pwd)
print(fr.model_dump_json(indent=2))
print(defaults)

Gives

{
  "type": "workflow",
  "inputs": [
    "x",
    "y",
    "a"
  ],
  "outputs": [
    "w"
  ],
  "nodes": {
    "foo_0": {
      "type": "atomic",
      "inputs": [
        "x",
        "y"
      ],
      "outputs": [
        "output_0"
      ],
      "reference": {
        "info": {
          "module": "__main__",
          "qualname": "foo",
          "version": null
        },
        "inputs_with_defaults": []
      },
      "unpack_mode": "tuple"
    },
    "bar_0": {
      "type": "atomic",
      "inputs": [
        "a",
        "b"
      ],
      "outputs": [
        "output_0"
      ],
      "reference": {
        "info": {
          "module": "__main__",
          "qualname": "bar",
          "version": null
        },
        "inputs_with_defaults": []
      },
      "unpack_mode": "tuple"
    }
  },
  "input_edges": {
    "foo_0.x": "x",
    "foo_0.y": "y",
    "bar_0.a": "a"
  },
  "edges": {
    "bar_0.b": "foo_0.output_0"
  },
  "output_edges": {
    "w": "bar_0.output_0"
  },
  "reference": null
}
{'x': 1, 'y': 2, 'a': 3}

Note that in the original recipe, we'd have been able to see inputs_with_defaults: ["x", "y"] for the foo atomic node recipe, but these get lost in round-tripping because there's nowhere to stick that information in the PWD representation. This doesn't make the roundtripped recipe wrong, just less complete in an acceptable way. Ultimately there are just some limitations in roundtripping like this, where we're limited for each piece of information by the minimal quality of information across transformed representations 🤷‍♂️

Footnotes

  1. This could be avoided by really trying to import the thing and then re-calculating its actual module+qualname, but I am not willing to go that far at the moment.

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
To keep things consistent everywhere. An alternative would be to pass a delimiter character around, but I like this.

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Now that we use it somewhere else.

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Binder 👈 Launch a binder notebook on branch pyiron/flowrep/pwd-converter

pyiron-runner and others added 3 commits March 6, 2026 23:36
It is only re-importing things for exposure. This should have gotten done in the API PR, but there was a server side hiccup where the tests never got ran and I didn't bother closing/reopening such a simple PR to run them once it was recovered.

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
@liamhuber
Copy link
Member Author

      The following packages are incompatible
      ├─ pydantic =2.12.5 * is requested and can be installed;
      └─ python-workflow-definition =0.1.3 * is not installable because it requires
         └─ pydantic >=2.7.0,<=2.12.4 *, which conflicts with any installable versions previously reported.

Pinning all the way to patch versions is inconvenient. Now I've got to downgrade my pydantic version just because python-workflow-definition hasn't upgraded yet. @jan-janssen, I don't suppose there's any chance I could convince you to relax the constraints on PWD going forward so that we don't need to coordinate cascading upgrades as frequently as we would with patch pins?

@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 98.95288% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.99%. Comparing base (94609b2) to head (7b7f359).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
flowrep/models/api/converters.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #175      +/-   ##
==========================================
+ Coverage   97.91%   97.99%   +0.08%     
==========================================
  Files          32       34       +2     
  Lines        2062     2248     +186     
==========================================
+ Hits         2019     2203     +184     
- Misses         43       45       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jan-janssen
Copy link
Member

@jan-janssen, I don't suppose there's any chance I could convince you to relax the constraints on PWD going forward so that we don't need to coordinate cascading upgrades as frequently as we would with patch pins?

As I currently do not have the dependabot pipeline in place for the python workflow definition, I extended the range to <2.13 . The tests pass so I think this is ready to be merged.

@liamhuber
Copy link
Member Author

As I currently do not have the dependabot pipeline in place for the python workflow definition, I extended the range to <2.13 .

🚀

The tests pass so I think this is ready to be merged.

Super. pydantic needs to be un-downgraded. I'm going to clean up the git history for the versions while I do that.

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
@liamhuber liamhuber merged commit d38a19e into main Mar 7, 2026
20 checks passed
@liamhuber liamhuber deleted the pwd-converter branch March 7, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants