Skip to content

kwargs#24

Merged
amniskin merged 3 commits into
masterfrom
kwargs
Oct 6, 2025
Merged

kwargs#24
amniskin merged 3 commits into
masterfrom
kwargs

Conversation

@amniskin
Copy link
Copy Markdown
Contributor

@amniskin amniskin commented Oct 5, 2025

No description provided.

@amniskin amniskin requested a review from Copilot October 6, 2025 00:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces support for keyword arguments (kwargs) to function calls in DaggerML, allowing functions to override prepopulated values dynamically at call time. The changes enable more flexible function execution by supporting kwargs that can override existing prepop values in Executable objects.

  • Support for kwargs in dag.call() and ExecutableNode.__call__() methods
  • Validation to ensure kwargs correspond to existing prepop keys
  • Consolidation of test fixtures to use a shared dml fixture

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/daggerml/core.py Adds kwargs support to call methods and improves node documentation handling
tests/test_core.py Refactors tests to use shared dml fixture and adds kwargs validation tests
tests/conftest.py Introduces pytest fixtures for DML testing infrastructure
CONTRIBUTING.md Adds documentation for pytest markers

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/daggerml/core.py Outdated
Comment thread src/daggerml/core.py
Comment on lines +493 to +494
# FIXME: replace fails: `TypeError: Executable.__init__() missing 1 required positional argument: 'uri'`
# fn = replace(fn, prepop={**fn.prepop, **kw})
Copy link

Copilot AI Oct 6, 2025

Choose a reason for hiding this comment

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

The commented FIXME indicates incomplete implementation. Consider either implementing the replace functionality properly or removing the comment if the current approach is intentional.

Suggested change
# FIXME: replace fails: `TypeError: Executable.__init__() missing 1 required positional argument: 'uri'`
# fn = replace(fn, prepop={**fn.prepop, **kw})

Copilot uses AI. Check for mistakes.
amniskin and others added 2 commits October 5, 2025 17:00
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@amniskin amniskin merged commit b93c461 into master Oct 6, 2025
14 checks passed
@amniskin amniskin deleted the kwargs branch October 6, 2025 00:07
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.

2 participants