dry-python/returns

Document methods.cond vs pointfree.cond

MajorDallas opened this issue · 3 comments

Bug report

What's wrong

In the docs for methods and pointfree, it's not immediately clear:

  1. That both modules have a function named cond
  2. What their relationship is and what their differences are

This first came to my attention while trying things out with ipython:

In[0]: from result.methods import *

In[1]: @safe
  ...: def func_using_cond(): ...

In[2]: func_using_cond()
Out[2]: <Success: 'foo'>

In[x]: from result.pointfree import *

In[x+1]: func_using_cond()
TypeError: cond() takes from 2 to 3 positional arguments but 4 were given

All in all, that's not such a big deal by itself. I'm only using import * because I'm in a repl and would never do this in code. Still, because I hadn't read all of pointfree's docs, I had no idea I was importing another cond and was very surprised when my previously-working function was suddenly broken.

How is that should be

My first thought was to rename the two functions: methods.cond4 and pointfree.cond3 to reflect their different arg counts. I suppose that is an option, but any library user can import cond as condx if that's what they prefer.

Instead, I think it would be enough that the docs for both functions at least mention the existence of the other and provide an example of how each interacts with a given context. Currently, both functions have decent examples, but since the two examples use completely different contexts one has to think hard about how the other function would change how the code must be written.

This experiment is what finally made it click for me how both can and should be used:

from returns.functions import compose
from returns.methods import cond as m_cond
from returns.pointfree import bind, cond as pf_cond
from returns.result import Failure, Result, Succes, safe

with_pfcond = compose(
    safe( lambda i: 1/i > 0 ),
    bind( pf_cond(Result, True, False) ),  # <-- pf_cond returns `(bool) -> Result`, which bind can call directly
)

with_mcond = compose(
    safe( lambda i: 1/i > 0 ),
    bind( lambda b: m_cond(Result, b, True, False) ),  # <-- m_cond returns `Result`, and so must be wrapped in a lambda for bind
)

for i in range(-1, 2):
    res = with_pfcond(i)
    match res:
        # Since two ZeroDivisionError instances are not equal:
        case Failure(ZeroDivisionError()):
            assert res.alt(type) == with_mcond(i).alt(type)
        case Success() | Failure():
            assert res == with_mcond(i)

After this experiment, I can easily see why pf_cond is the better choice when using flow (or compose), but just seeing pf_cond used with flow in its example wasn't enough to help me understand why.

System information

  • python version: 3.11
  • returns version: 0.22.0
  • mypy version: 1.5.1

Thanks a lot for the detailed description. I understand the problem. I think that updating docs is the way to go here. We totally should mention them both here: https://returns.readthedocs.io/en/latest/pages/methods.html#cond and here: https://returns.readthedocs.io/en/latest/pages/pointfree.html#cond

I also think that we can change the docs to advertise from returns import pointfree; pointfree.cond(...) and from returns import methods; methods.whatever(...) style, because this is the most convenient way to do this.

Whould you like to contribute? :)

I can take a look at adding a line for each of those.

I am thinking simply adding a .. note:: that points to the other function and change the imports as you explained :)