Skip to content

Conversation

@samwaseda
Copy link
Member

First item in the list of #735

@samwaseda samwaseda requested a review from liamhuber July 29, 2025 14:37
@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_workflow/mkdir

@codecov
Copy link

codecov bot commented Jul 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.74%. Comparing base (611394c) to head (dc00689).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #736      +/-   ##
==========================================
+ Coverage   92.29%   92.74%   +0.44%     
==========================================
  Files          34       34              
  Lines        3726     3694      -32     
==========================================
- Hits         3439     3426      -13     
+ Misses        287      268      -19     

☔ 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.

@samwaseda
Copy link
Member Author

I thought this was a pretty neat implementation, but then with the following code I encountered a problem:

from pyiron_workflow import Workflow

import uuid
from pathlib import Path

@Workflow.wrap.as_function_node
def generate_working_directory(path):
    working_directory = Path(path) / Path(uuid.uuid4().hex)
    working_directory.mkdir(exist_ok=True, parents=True)
    return working_directory

@Workflow.wrap.as_function_node
def write_hello(working_directory, file_name="my_file.txt"):
    full_path = Path(working_directory) / Path(file_name)
    with open(full_path, "w") as f:
        f.write("hello")
    return full_path

@Workflow.wrap.as_macro_node
def example_macro(macro, file_name="my_file.txt"):
    path = macro.as_path(mkdir=True)
    macro.working_directory = generate_working_directory(path)
    macro.message = write_hello(working_directory=macro.working_directory, file_name=file_name)
    return macro.message

wf = Workflow("wf")
wf.macro = example_macro()

This created a directory called example_macro, and not wf/example_macro. Is there a way to map the full hierarchy? @liamhuber

@samwaseda samwaseda marked this pull request as draft July 29, 2025 14:49
@liamhuber
Copy link
Member

This created a directory called example_macro, and not wf/example_macro. Is there a way to map the full hierarchy? @liamhuber

Indeed it did. Code decorated with as_macro_node is run at instantiation. Your node example_macro() only gets parented to the workflow at assignment.

I don't see anything wrong with the code you add in this PR, but I don't think it solves the problem you're trying to solve the way you want to solve it.

I like generate_working_directory, and it's a good candidate for the standard library. By always requiring the path and providing it later, we can get this example to work as you intended without ever using mkdir:

from pyiron_workflow import Workflow

import uuid
from pathlib import Path

@Workflow.wrap.as_function_node
def generate_working_directory(path):
    working_directory = Path(path) / Path(uuid.uuid4().hex)
    working_directory.mkdir(exist_ok=True, parents=True)
    return working_directory

@Workflow.wrap.as_function_node
def write_hello(working_directory, file_name="my_file.txt"):
    full_path = Path(working_directory) / Path(file_name)
    with open(full_path, "w") as f:
        f.write("hello")
    return full_path

@Workflow.wrap.as_macro_node
def example_macro(macro, path, file_name="my_file.txt"):
    macro.working_directory = generate_working_directory(path)
    macro.message = write_hello(working_directory=macro.working_directory, file_name=file_name)
    return macro.message

wf = Workflow("wf")
wf.macro = example_macro()
wf.macro.inputs.path = wf.macro.as_path()
wf()
>>> {'macro__message': PosixPath('/.../wf/macro/7fabb56f040645bca06f139ec7bb09cf/my_file.txt')}

@samwaseda samwaseda marked this pull request as ready for review July 29, 2025 16:19
@samwaseda
Copy link
Member Author

ok then maybe the current one is pretty ok, so I made it now again ready for review

Copy link
Member

@liamhuber liamhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samwaseda there is absolutely nothing wrong with this, but I don't yet see how we will use it so my brain is waving a YAGNI yellow flags are waving.

@samwaseda
Copy link
Member Author

I don't yet see how we will use it so my brain is waving a YAGNI yellow flags are waving.

ok let's see how people are gonna react, but I'm relatively sure that people will like to have a straightforward way to get a folder where they can freely store whatever they want. At the same time it was a bit confusing for me that as_path gives me a path, but the path actually doesn't exist, so even if the default behavior is going to be the same, I think it makes sense that there's a way to make the path exist. Anyway since you approved it I merge it 😎

Force delete with `delete_even_if_not_empty`
@samwaseda samwaseda merged commit 4d7c474 into main Jul 29, 2025
5 checks passed
@samwaseda samwaseda deleted the mkdir branch July 29, 2025 16:47
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