search

chalk-diagrams / chalk

A declarative drawing API in Python
MIT License
282 stars 13 forks source link

Associative Reduce #129

Closed srush closed 8 months ago

srush commented 8 months ago

Hey @danoneata! Long time!

I've been using chalk for a new project and I realized that functools is really dumb about reductions. It doesn't make any assumption about the associativity of the operator which means we are recursing to an O(N) depth with the functional style not O(log(N)). This change fixes the problem with no change to the underlying code.

danoneata commented 8 months ago

Hello Sasha! I'm very happy to hear from you! I've noticed that you are still making good use of chalk in your side projects. I feel a bit guilty that I couldn't keep up with the active development of the library, but I still hope to find some free time to start addressing some of the issues.

Thanks for the PR! Yes, efficiency is a sore point of the current implementation, so using the log(N) variant of reduce is a good step in the right direction.

srush commented 8 months ago

Honestly it mostly works well for my use cases! Speed is mostly not a problem.