tawazi icon indicating copy to clipboard operation
tawazi copied to clipboard

[BUG] Edge case for the unpack_to argument

Open matthiasmindee opened this issue 1 year ago • 13 comments

from tawazi import xn

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

this leads to unexpected behaviours

matthiasmindee avatar Apr 13 '23 13:04 matthiasmindee

what is the expected behavior and what is happening ?

bashirmindee avatar Jul 18 '23 11:07 bashirmindee

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

matthiasmindee avatar Jul 18 '23 14:07 matthiasmindee

from tawazi import xn, dag

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

@dag
def pipe():
    return toto()

pipe()

This doesn't hang locally

bashirmindee avatar Jul 18 '23 14:07 bashirmindee

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__'

bashirmindee avatar Jul 18 '23 14:07 bashirmindee

well that's weird

matthiasmindee avatar Jul 18 '23 16:07 matthiasmindee

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 avatar Jul 18 '23 16:07 bashirmindee

what action do you think should be taken ?

bashirmindee avatar Jul 18 '23 16:07 bashirmindee

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

matthiasmindee avatar Jul 19 '23 08:07 matthiasmindee

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

bashirmindee avatar Jul 19 '23 08:07 bashirmindee

yes that exactly my point ahahah

matthiasmindee avatar Jul 19 '23 08:07 matthiasmindee

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

bashirmindee avatar Jul 19 '23 08:07 bashirmindee

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

matthiasmindee avatar Jul 19 '23 08:07 matthiasmindee

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 avatar Jul 19 '23 08:07 bashirmindee

stale

bashirmindee avatar Jul 25 '24 15:07 bashirmindee