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

BUG: task.execute returns None despite the function returning a value #547

Closed
2 of 3 tasks
Ostheer opened this issue Jan 11, 2024 · 5 comments
Closed
2 of 3 tasks
Labels
bug Something isn't working

Comments

@Ostheer
Copy link
Contributor

Ostheer commented Jan 11, 2024

  • I have checked that this issue has not already been reported.
  • I have confirmed this bug exists on the latest version of pytask.
  • (optional) I have confirmed this bug exists on the main branch of pytask.

Code Sample, a copy-pastable example

import pytask
from pathlib import Path
from bandgetouw.config import BLD

for id_ in ("unique_id", "another_one"):
    def some_fun(some_data):
        assert isinstance(some_data, str)
        print(some_data)
        return some_data

    pytask.task(
        id=id_, 
        produces=Path(BLD / "test" / f"{id_}.txt"),
        kwargs={"some_data": "stuff"}
    )(some_fun)

Problem description

This is the output when I run pytask -s:

Collected 10 tasks.

stuff
stuff
╭─────────────────────────────────────┬─────────╮
│ Task                                │ Outcome │
├─────────────────────────────────────┼─────────┤
│ task_test.py::some_fun[another_one] │ F       │
│ task_test.py::some_fun[unique_id]   │ F       │
╰─────────────────────────────────────┴─────────╯

──────────────────────────────────────── Failures ────────────────────────────────────────

──────────────────── Task task_test.py::some_fun[another_one] failed ─────────────────────

TypeError: 'PathNode' can only save 'str' and 'bytes', not <class 'NoneType'>

───────────────────── Task task_test.py::some_fun[unique_id] failed ──────────────────────

TypeError: 'PathNode' can only save 'str' and 'bytes', not <class 'NoneType'>

──────────────────────────────────────────────────────────────────────────────────────────
╭──────────────── Summary ─────────────────╮
│  10  Collected tasks                     │
│  8   Skipped because unchanged  (80.0%)  │
│  2   Failed                     (20.0%)  │
╰──────────────────────────────────────────╯
───────────────────────────────── Failed in 0.03 seconds ─────────────────────────────────

Expected Output

The tasks should succeed and the PathNode should've save "stuff" to the two output files.
Instead, PathNode.save fails because it cannot save the NoneType.

I've traced back the problem to the execute method of the Task class in _pytask.nodes:
There, the function (in this case some_fun) is called: return self.function(**kwargs).
I inspected the return value of self.function(**kwargs)), and it is indeed None. Obviously, I was expecting that value to be the string from some_fun.

In my particular case, my function is a simple lambda wrapper around a function (related to this issue) defined elsewhere called xrn_task. I have been able to work around this issue with the following hack in the Task class:

    def execute(self, **kwargs: Any) -> None:
        """Execute the task."""
        if "xrn_task" in inspect.getsource(self.function):
            from bandgetouw.tasks import xrn_task
            return xrn_task(**kwargs)
        else:
            return self.function(**kwargs)

Additionally, I'm not sure why the return type hint of the execute method is None.

@tobiasraabe
Copy link
Member

tobiasraabe commented Jan 11, 2024

Hi @Ostheer,

thanks for taking the time to file the issue.

Your reproducible example may not be complete because I can run it. I slightly changed it since I do not know about the module bandgetouw.

import pytask
from pathlib import Path

for id_ in ("unique_id", "another_one"):
    def some_fun(some_data):
        assert isinstance(some_data, str)
        print(some_data)
        return some_data

    pytask.task(
        id=id_,
        produces=Path(f"{id_}.txt"),
        kwargs={"some_data": "stuff"}
    )(some_fun)

You are entirely correct about the type hints. They should be Any.

@Ostheer
Copy link
Contributor Author

Ostheer commented Jan 11, 2024

Hey @tobiasraabe,

Thank you for your response.

Your edit to the example code does not affect the nature of the symptom. The bandgetouw module is irrelevant here and just supplied my build directory.

Have you been able to reproduce the issue with your modified code?

@tobiasraabe
Copy link
Member

Sorry if I was unclear. If I run the adapted code, the two files are produced. Everything works.

This is why I asked whether something else is happening with your problem that is not reflected in your reproducible example.

Have you run the adapted example? Does it throw an error on your side?

@Ostheer
Copy link
Contributor Author

Ostheer commented Jan 11, 2024

It did throw the same error indeed. Because of the None type hint I started to doubt if I was even correct in expecting the function there to return anything.

I just created a new virtual environment and that somehow solved the issue (despite having already upgraded pytask and all its deps).

In any case, it works now. My only suggestion then is that type hint :).

Thanks for checking in and keep up the good work!

@tobiasraabe
Copy link
Member

I am glad it is working now for you. #548 fixes the issue you mentioned. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants