Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the daggerml dependency and adds delayed functionality to the experimental API. The changes enable deferred execution of field initialization and method calls within DAG objects, allowing for more flexible DAG construction patterns.
- Upgraded daggerml from version 0.0.38.post2 to 0.0.38.post3
- Added
DelayedActionclass and related functionality for deferred execution in the experimental API - Enhanced test coverage with new unit tests for delayed functionality and added proper test markers
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/dml_util/experimental/api.py | Core implementation of DelayedAction class, delayed decorator, and enhanced Dag class with context manager support |
| tests/unit/experimental/test_api.py | New comprehensive unit tests for delayed functionality including field defaults and load operations |
| pyproject.toml | Dependency version upgrade and new test marker configuration |
| tests/conftest.py | Enhanced debug fixture with logging configuration |
| .github/workflows/ci.yml | CI workflow updates to exclude DML-dependent tests |
| tests/integration/ | Updated test markers for proper test categorization |
| src/dml_util/funk.py | Added logging imports and logger setup |
| tests/assets/sum.py | New test asset file for integration testing |
| submodules/python-lib | Submodule commit hash update |
| method = funk(method) | ||
| assert isinstance(method, Executable) | ||
| method.prepop.update({k: dag[k] for k in deps[method_name] if hasattr(dag, k)}) | ||
| dag.put(method, name=method_name) |
There was a problem hiding this comment.
The method is being put into the DAG twice on consecutive lines. Line 224 should be removed as line 225 already handles putting the method into the DAG and setting the attribute.
| dag.put(method, name=method_name) |
| # if isinstance(method, Node): | ||
| # method = method.value() |
There was a problem hiding this comment.
Remove commented-out code. If this logic might be needed in the future, consider adding a TODO comment explaining when it would be used.
| # if isinstance(method, Node): | |
| # method = method.value() |
No description provided.