suned / pfun

Functional, composable, asynchronous, type-safe Python.
https://pfun.dev/
MIT License
151 stars 14 forks source link

List::append() TypeError #39

Closed thomhickey closed 4 years ago

thomhickey commented 4 years ago

From the doc string:

List(range(3)).append(3)

this generates a TypeError because append only takes an Iterable. Am I missing something?

thomhickey commented 4 years ago

I must be doing something wrong, I also can't:

List(range(3)).reduce(sum)

gives:

TypeError: 'NoneType' object is not iterable

the problem is with the sum function, doesn't work as an argument to List::reduce() nor does it work as an argument to functools reduce, you have to use lambda a, b: a + b

I'm no Python expert, so it's probably me.

suned commented 4 years ago

The problem here is that sum doesn't have the correct signature: it takes an iterable and performs a reduce. The function you are looking for is operator.add

thomhickey commented 4 years ago

Yes, thanks, I know I can find other functions that will work. I was taking those examples from your doc strings, lines 39 and 53 of list.py. Do you want me to PR some fixes that use different examples? They both also appear in the readthedocs guide.

thomhickey commented 4 years ago

also, my first issue reported here is this:

List(range(3)).append(3)

this doesn't work either, seems broken. definitely need to be able to add a single element to a list.

suned commented 4 years ago

I was taking those examples from your doc strings, lines 39 and 53 of list.py

Ah, my mistake :)

Do you want me to PR some fixes that use different examples? They both also appear in the readthedocs guide.

That would be awesome

List(range(3)).append(3)

Guess the documentation here is bit outdated as well. The current behaviour comes from the connection to Monoid where append is an alias for the + operator, meaning they should be the same. Maybe it would make sense to special case append for List to accept both an iterable and an element since the latter is the normal behaviour for python lists

suned commented 4 years ago

Added #41 to fix that :)

thomhickey commented 4 years ago

Awesome! Thanks so much. I reviewed and approved the changes, lol. Not sure if you want me to contribute here or not :)

I work at Genentech for the People Analytics team. I'm looking for an FP library to use and like I said I like yours the most, but I'm not sure if you consider it production ready? Thanks again!

suned commented 4 years ago

I'm looking for an FP library to use and like I said I like yours the most, but I'm not sure if you consider it production ready?

Cool 😄 ! In terms of 'production readiness', I suppose it depends on what you need from it. I think the api is pretty much stable, and the current implementation is reasonably well tested (feel free to take a critical look at the test suite :))

The main shortcoming at this point is performance: there is quite a bit of overhead of calling functions and iterating over lists in python, which makes the monads that require Trampoline for stack safety somewhat slow. I'm working on porting everything here to Cython to fix that, but am currently blocked by https://github.com/cython/cython/milestone/58.

Also as you have already discovered, the documentation is not 100% up to date.

So if you don't need to chain together hundreds of thousands of trampolined monadic actions, and you can live with an occasional documentation issue, you should be ok :)

thomhickey commented 4 years ago

Thanks so much, Sune. I was busy on another project for a few weeks there, but will be returning to this some time soon. All of our projects so far are light in load and scale, so the performance issues wouldn't present a problem especially if there are improvements coming in the future.

suned commented 4 years ago

Fixed in version 0.6.0