mindee / tawazi

A DAG Scheduler library written in pure python
https://mindee.github.io/tawazi/
Apache License 2.0
70 stars 5 forks source link

[BUG] Edge case for the unpack_to argument #123

Closed matthiasmindee closed 2 months ago

matthiasmindee commented 1 year ago
from tawazi import xn

@xn(unpack_to=1)
def toto():
    ...

this leads to unexpected behaviours

bashirmindee commented 1 year ago

what is the expected behavior and what is happening ?

matthiasmindee commented 1 year ago

this makes the execution freeze. The expected behaviour would either be to trigger an exception saying that there's nothing to unpack, or not try to unpack the result

bashirmindee commented 1 year ago
from tawazi import xn, dag

@xn(unpack_to=1)
def toto():
    ...

@dag
def pipe():
    return toto()

pipe()

This doesn't hang locally

bashirmindee commented 1 year ago

I am getting this:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/bashir/mindee/tawazi/tawazi/_dag/dag.py", line 674, in __call__
    return self._get_return_values(all_nodes_dict)  # type: ignore[return-value]
  File "/home/bashir/mindee/tawazi/tawazi/_dag/dag.py", line 736, in _get_return_values
    return tuple(gen)
  File "/home/bashir/mindee/tawazi/tawazi/_dag/dag.py", line 734, in <genexpr>
    gen = (ren_uxn.result(xn_dict) for ren_uxn in return_uxns)
  File "/home/bashir/mindee/tawazi/tawazi/node/node.py", line 639, in result
    return _filter_noval(reduce(lambda obj, key: obj.__getitem__(key), self.key, xn.result))
  File "/home/bashir/mindee/tawazi/tawazi/node/node.py", line 639, in <lambda>
    return _filter_noval(reduce(lambda obj, key: obj.__getitem__(key), self.key, xn.result))
AttributeError: 'NoneType' object has no attribute '__getitem__'
matthiasmindee commented 1 year ago

well that's weird

bashirmindee commented 1 year ago

yes but it doesn't hang... The problem here is that pass means that the function doesn't return a Tuple... It returns None! This is why we have this error. For Example if you write this, the code works as expected:

from tawazi import xn, dag

@xn(unpack_to=1)
def toto():
    return (1,)

@dag
def pipe():
    v, = toto()
    return v

assert 1 == pipe()
bashirmindee commented 1 year ago

what action do you think should be taken ?

matthiasmindee commented 1 year ago

yes but

@xn(unpack_to=1)
def toto():
    return 1

fails

I think we should actually ban unpack_to=1 since this is most probably unwanted by the user

bashirmindee commented 1 year ago

of course it fails because 1 is not a tuple...

matthiasmindee commented 1 year ago

yes that exactly my point ahahah

bashirmindee commented 1 year ago

Python allows for single valued Tuples... This is why it is supported. I agree that I don't see the point of having a tuple which contains a single value but this is Python XD

matthiasmindee commented 1 year ago

yeah I'd suggest banning that altogether since it's error-prone anyways

bashirmindee commented 1 year ago

I think we should have at least a clearer error message. Because even if we replace unpack_to=1 by unpack_to=2 we get the same error message. So I believe the problem lies in the error message more than in the unpack_to=1...

bashirmindee commented 2 months ago

stale