Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

%load_node support full Kedro Node syntax, with better *args, **kwargs handling #3629

Closed
Tracked by #3580 ...
noklam opened this issue Feb 19, 2024 · 0 comments · Fixed by #3633
Closed
Tracked by #3580 ...

%load_node support full Kedro Node syntax, with better *args, **kwargs handling #3629

noklam opened this issue Feb 19, 2024 · 0 comments · Fixed by #3633
Assignees

Comments

@noklam
Copy link
Contributor

noklam commented Feb 19, 2024

Copy from #3580 (comment)

Better handle of *args and **kwargs, currently the %load_node have a simple logic to map node's input to function parameters, it fails with edge cases. Kedro node syntax: https://docs.kedro.org/en/latest/nodes_and_pipelines/nodes.html#syntax-for-input-variables

The idea is to use inspect.Signature.bind and inspect.Parameters. Additionally, we can identify the special arguments (VAR_POSITIONAL) if needed.

We are already using similar logic to validate node inputs, so this should make sure we support valid inputs only.

def _validate_inputs(
self, func: Callable, inputs: None | str | list[str] | dict[str, str]
) -> None:
# inspect does not support built-in Python functions written in C.
# Thus we only validate func if it is not built-in.
if not inspect.isbuiltin(func):
args, kwargs = self._process_inputs_for_bind(inputs)
try:
inspect.signature(func, follow_wrapped=False).bind(*args, **kwargs)
except Exception as exc:

For example:

def dummy(a,b,c, *args, **kwargs):
    ...

node(dummy, ["data_1", "data_2", "data_3", "dummy1","dummy2","dummy3"], ...)

should translate to

a = catalog.load("data_1")
b = catalog.load("data_2")
c = catalog.load("data_3")
dummy1 = catalog.load("dummy1")
dummy2 = catalog.load("dummy2")
dummy3 = catalog.load("dummy3")
args = [dummy1, dummy2, dummy3] # Noted here the name of the "dummy_x" variable are arbitrary
dummy(a, b, c, *args)

Requirements

  • Include richer sets of test case that support all node input syntax that kedro support.

Supplementary

  • Support optional arg for %load_node #3623 is a marginal improvement, but I found it will be difficult to have generic support. This issue propose to use the inspect.Signature.bind method to get generic support
@noklam noklam assigned noklam and unassigned noklam Feb 19, 2024
@merelcht merelcht moved this to To Do in Kedro Framework Feb 19, 2024
@noklam noklam moved this from To Do to In Progress in Kedro Framework Feb 19, 2024
@noklam noklam changed the title Handle *args, **kwargs better for %load_node %load_node support full Kedro Node syntax, with better *args, **kwargs handling Feb 19, 2024
@noklam noklam linked a pull request Feb 19, 2024 that will close this issue
7 tasks
@merelcht merelcht mentioned this issue Feb 19, 2024
4 tasks
@lrcouto lrcouto moved this from In Progress to In Review in Kedro Framework Feb 23, 2024
@github-project-automation github-project-automation bot moved this from In Review to Done in Kedro Framework Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants