dbrattli / Expression

Pragmatic functional programming for Python inspired by F#
https://expression.readthedocs.io
MIT License
421 stars 30 forks source link

change function for map does not appear to work correctly #105

Closed jaycunningham-8451 closed 5 months ago

jaycunningham-8451 commented 1 year ago

Describe the bug This may not be a bug, but I can't find documentation for the change function / method as it applies to maps. It certainly does not appear to operate in a way that I would expect.

Calling change on an existing, nonempty map appears to operate as if it were called on an empty map.

To Reproduce

This results in a Nothing_ error, when I would expect it to return the map unchanged:

from expression.collections import Map

xs = Map.of(**dict(a=10, b=20))
xs.change("a", lambda x: x)

This results in map [("a", 1)] when I would expect map [("a", 1); ("b", 20)]:

xs.change("a", lambda _: Some(1))

Expected behavior As indicated above, I would expect that the first block would return the map unchanged, and that the second would only modify the item with the key "a".

Additional context Add any other context about the problem here.

matiboy commented 1 year ago

I'm facing the same issue; a bit more specifically, the map can only change thesecond item added:

def test_expression_issue_105():
    m = Map.empty()
    m = m.add("1", 1).add("2", 2).add("3", 3).add("4", 4)
    print(m)  # map [("1", 1); ("2", 2); ("3", 3); ("4", 4)]
    m = m.change("2", lambda x: x)  # <- works cause "2" is second added item
    print(m)  # map [("1", 1); ("2", 2); ("3", 3); ("4", 4)] All keys still there
    # m = m.change("1", lambda x: x)  # fails (x is Nothing)
    m = m.change("3", lambda x: x)  # fails (x is Nothing)
matiboy commented 1 year ago

I noticed that the code in maptree.py lines 315 and 328 uses rebalance but does not return it which seems wrong since that would mean it has side effects (right?)

Adding return rebalance(... makes the tests I wrote pass, and all other tests still pass too. I'll create a PR but hopefully the authors/maintainers can have a close look because I know very little about MapTrees and generally functional programming.