Better error when calling len() on a reactive expression

[maximlt]

Closes #954

The suggestion in the issue was:

I believe we can't implement __len__ reactively, but can't we provide a better error message in this case? E.g. it seems like we could print a warning saying that the value returned won't be reactive (and what to do instead (.rx.len()), and then return the non-reactive length.

I am not in favor of emitting a warning and returning the non-reactive length as this sets a bit of a weird precedent (we could do the same for other parts of the Python data model we can't proxy). Instead with this PR a TypeError is raised with a more explicit message. Note that if in the future we decide to warn and return the non-reactive length, that PR makes it still possible.

TypeError: len(<rx_obj>) is not supported. Use `<rx_obj>.rx.len()` to obtain
the length as a reactive expression, or `len(<rx_obj>.rx.value)` to obtain the
length of the underlying expression value.

After implementing that, this test started to fail:

def test_reactive_len():
    i = rx([1, 2, 3])
    l = i.rx.len()
    assert l.rx.value == 3
    i.rx.value = [1, 2]
    assert l == 2

It turns out that:

1. it was buggy, the last assert should have been assert l.rx.value == 2. l == 2 returns a reactive expression that is truthy
2. truth value testing checks for __bool__ first and __len__ second. Since rx did not implement __bool__ and this PR added __len__ with a TypeError, calling bool(<rx_obj>) (or if <rx_obj>: ...) started to emit an error.

This PR resolves that by:

1. Fixing the test
2. Implementing rx.__bool__ that always returns True like it used to before the PR.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.28%. Comparing base (facb9e5) to head (0e8c0a5).
Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1033      +/-   ##
==========================================
+ Coverage   87.27%   87.28%   +0.01%     
==========================================
  Files           9        9              
  Lines        4928     4932       +4     
==========================================
+ Hits         4301     4305       +4     
  Misses        627      627              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jbednar]

Looks good! This seems like excellent feedback to a user, telling them precisely what they need to know ("hey -- this isn't going to work! Did you mean a, or b, both of which would work, but they're different"). Nice!

Can we be similarly helpful for the other unimplementable bits of the rx space?

[maximlt]

Can we be similarly helpful for the other unimplementable bits of the rx space?

I'm not sure to be honest and none comes to mind from my experience of using rx, that is in fact quite limited.

[philippjfr]

The only thing we could realistically do is error on __bool__ because most other operators like and, or are not overridable and are simply downstream from bool. I'm not 100% convinced either way since always returning True is also a valid interpretation and avoids users having to write rx_obj is None instead.

[maximlt]

Yes I thought about __bool__ too, raising an error would have prevented the bug in the test suite fixed in this PR (https://github.com/holoviz/param/pull/1033/files#diff-6b57f1fb19e9ef0cec1c2c4f6d9ecba42bca040b660a1ee6b19cfdb8ad0dd049R296). However, like you, I'm also not sure it's a good idea as objects that raise an error on truth testing are often sentinels (e.g. _Undefined in Param). A more common example is Numpy/Pandas objects with e.g. ValueError: The truth value of a Series is ambiguous. Use a.empty, a.bool(), a.item(), a.any() or a.all(). I guess in the end I'd be fine if __bool__ was unconditionally raising an error but I won't implement it in this PR (it'd probably need a little deprecation period).