Add Flow.metadata attribute and Flow.update_metadata method

[janosh]

Flow.update_metadata is now much more similar to and has reached feature parity with Job.update_metadata:

jobflow/src/jobflow/core/job.py

Lines 922 to 930 in 367836b

	def update_metadata(
	self,
	update: dict[str, Any],
	name_filter: str = None,
	function_filter: Callable = None,
	dict_mod: bool = False,
	dynamic: bool = True,
	):
	"""

[codecov]

Codecov Report

Attention: Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.18%. Comparing base (a740c6c) to head (b6786ca).
Report is 52 commits behind head on main.

Files with missing lines	Patch %	Lines
src/jobflow/core/flow.py	92.30%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #679      +/-   ##
==========================================
- Coverage   99.23%   99.18%   -0.06%     
==========================================
  Files          21       21              
  Lines        1573     1590      +17     
  Branches      427      339      -88     
==========================================
+ Hits         1561     1577      +16     
  Misses         10       10              
- Partials        2        3       +1     

Files with missing lines	Coverage Δ
src/jobflow/core/job.py	99.15% <100.00%> (+<0.01%)	⬆️
src/jobflow/managers/local.py	100.00% <ø> (ø)
src/jobflow/core/flow.py	99.55% <92.30%> (-0.45%)	⬇️

... and 2 files with indirect coverage changes

---- 🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[janosh]

@utf i'm sure you're strapped for time but even partial feedback here would be much appreciated!

[utf]

Thanks @janosh. My only concern is about breaking the API of update_metadata between Flows and Jobs. If we add the target option to Job.update_metadata that should make this function easier to use and the implementation cleaner.

[utf]

Here job could actually be a nested flow. So you should still iterate over these if target = "flow".

[janosh]

perhaps the naming could be improved. target = "flow" isn't meant to update the metadata of all nested flows (to the exclusion of jobs) but rather only the flow itself, not any of its jobs or nested flows.

how about we rename from target: Literal["flow", "jobs", "both"] = "both" to target: Literal["self", "nested", "both"] = "both"? unless you think it's important to have more control, i.e. be able to only update nested Flow or Job metadata. there might be use cases for that. in which case maybe we want target: Literal["self", "nested", "jobs", "flows", "both"] = "both"?

[janosh]

to clarify this could be the doc string:

        target
            Specifies where to apply the metadata update. Options are:

            - "self": Update only the Flow's own metadata
            - "nested": Update only the metadata of Jobs/Flows within the Flow
            - "jobs": Update only the metadata of Jobs within the Flow
            - "flows": Update only the metadata of Flows within the Flow
            - "all": Update both the Flow's metadata and nested Job+Flow metadata (default)

[utf]

I think this is overcomplicating it. If you want to select specific flows or jobs then you can pass in a name or class filter. So no need for the extra options. The main thing is the API should be consistent between jobs/flows.

[janosh]

makes sense. then the easiest thing would be to get rid of target altogether and use the "all" behavior?

[utf]

That's an interesting point. I think the benefit of target is that it would be a shortcut for class=Flow vs class=Job. I would be happy either having the target option or specifying the shortcut in the docstring.

[janosh]

different issue but while we're on the topic of API design, what's your take on adding a callback_filter: Callable[[Flow | Job], bool]? users would pass in a function which takes the Flow or Job instance on which you invoke update_metadata (or update_config, ...) and returns True if updates should be applied. perhaps more prone to user error but also very versatile. usage example:

Flow().update_metadata(
    {"material_id": 42},
    callback_filter=lambda flow: SomeMaker in map(type, flow)
    and flow.name == "flow name"
)

[janosh]

I think the benefit of target is that it would be a shortcut for class=Flow vs class=Job. I would be happy either having the target option or specifying the shortcut in the docstring.

i don't follow. by class=Job|Flow, did you mean filter_function=Job|Flow? the job variant filter_function=Job doesn't seem to work the way you're suggesting so maybe you meant sth else?

EDIT: i guess you meant class_filter on Maker.update_kwargs but that isn't implemented for Flow/Job.update_metadata

[utf]

Yes, I was thinking of class_filter. I think the callback_filter you proposed sounds very flexible and would be useful!

[utf]

@janosh I lost track of this. Is it good to go?

[janosh]

Not quite but Orion just pinged me about it so will try to pick it up again

[janosh]

@utf i think this is ready to go now. a future PR could add the callback_filter keyword to other update methods like update_kwargs as well

[janosh]

hmmm... added a lot of test cases. not sure what else to add. could it be the coverage calculation is off?

[utf]

@janosh just one comment. Happy for you to merge once that's done. I can then push a new version.

[utf]

Please can you add docstrings for these options in the class docstring? E.g. see the corresponding docs for Job.