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

Add explanation for arguments passing #1746

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions digdag-docs/src/operators/py.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,112 @@ See [Python API documents](../python_api.html) for details including variable ma
print("simple execution")
```

You can pass arguments to class for initialization using `_export` as the following:
yoyama marked this conversation as resolved.
Show resolved Hide resolved

```yaml
# sample.dig
+some_task:
_export:
required1_1: awesome execution
required1_2: "awesome execution"
required2: {a: "a"}
required3: 1
required4: 1.0
required5: [a, 1, 1.0, "a"]
py>: tasks.MyWorkflow.my_task
```

Also, you can do the same thing by defining arguments under the `py>:` operation as the following:
yoyama marked this conversation as resolved.
Show resolved Hide resolved
```yaml
# sample.dig
+some_task:
py>: tasks.MyWorkflow.my_task
required1_1: awesome execution
required1_2: "awesome execution"
required2: {a: "a"}
required3: 1
required4: 1.0
required5: [a, 1, 1.0, "a"]
```

```python
Copy link
Contributor

Choose a reason for hiding this comment

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

This example does not work by copy and paste it.
Don't we need to import Union as follows ?

from typing import Union

class MyWorkflow(object):
...

# tasks.py
class MyWorkflow(object):
def __init__(
self,
required1_1: str,
required1_2: str,
required2: dict[str, str],
required3: int,
required4: float,
required5: list[Union[str, int, float]]
):
print(f"{required1_1} same as {required1_2}")
self.arg2 = required2
print(f"{float(required3)} same as {required4}")
self.arg5 = required5

def my_task(self):
pass
```

Or, you can pass arguments to function as the following:

```yaml
# simple_sample.dig
+some_task:
_export:
required1: simple execution
required2: {a: "a"}
py>: simple_tasks.my_func
```

```yaml
# sample.dig
+some_task:
py>: simple_tasks.my_func
required1: simple execution
required2: {a: "a"}
```

```python
# simple_tasks.py
def my_func(required1: str, required2: dict[str, str]):
print(f"{required1}: {required2}")
```

Finally, you can pass combination of class and mehtod arguments to Python script as the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

This behavior is a bit strange.
IIUC, we can't use same argument name between class initializer and method argument for it.
It might be better to add a warning about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for my poor English. Just to be clear, what I tried to explain is in digdag, indeed we can pass both(class and method) arguments at once.
So, on-premise that the above behavior is correct what I understood is I just need to add a warning like ‘But you can not pass combination with the same name.’ instead of modifying the example.
Is this correct? @yoyama

Copy link
Contributor

Choose a reason for hiding this comment

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

@magicpieh28 Sorry for my comment is not clear.
Yes, your description and example are correct. But I feel a bit strange on this specification on py>.
Especially, if both initializer of Class and the method have same parameter name, we can't set them separately. (If I understand correctly).
So I propose add description to warn about it as you describe.
Anyway thank you for your contribution.


```yaml
# sample.dig
+some_task:
_export:
required_class_arg: awesome execution
required_method_arg: ["a", "b"]
py>: tasks.MyWorkflow.my_task
```

```yaml
# sample.dig
+some_task:
py>: tasks.MyWorkflow.my_task
required_class_arg: awesome execution
required_method_arg: ["a", "b"]
```

This example assume following Python script:

```python
# tasks.py
class MyWorkflow:
def __init__(self, required_class_arg: str):
self.arg = required_class_arg

def my_task(self, required_method_arg: list[str]):
print(f"{self.arg}: {required_method_arg}")
```


* **python**: PATH STRING or COMMAND ARGUMENTS LIST

The python defaults to `python`. If an alternate python and options are desired, use the `python` option.
Expand Down
6 changes: 6 additions & 0 deletions examples/python_args.dig
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,9 @@ timezone: UTC
key1: "a"
key2: "val2"

+e:
_export:
key1: "a"
key2: {b: "c"}
py>: tasks.python_args.exported_arguments