jaspervdj / psqueues

Priority Search Queues in three different flavors for Haskell
https://hackage.haskell.org/package/psqueues
Other
64 stars 24 forks source link

Small additions #5

Closed ariep closed 3 years ago

ariep commented 9 years ago

Hi,

For personal use, I added instances of Generic and Typeable of the types in psqueues. I also exposed the internal modules, so I could derive other (serialisation) instances.

Finally, I added two small functions to Data.OrdPSQ: 'takeMin' to take a list of given length of the least elements of the queue, and 'insertWith'.

jaspervdj commented 9 years ago

Thanks for the additions, I really appreciate it!

Two things I would like to see to get this merged:

Let me know if you need help with this.

ariep commented 9 years ago

OK, I have done that.

The 'takeMin' function is easy to give in terms of the existing 'minView'.

The 'insertWith' for IntPSQ and HashPSQ I have implemented in terms of the 'alter' function. I hope this is OK; it should give at least the right complexity. Because OrdPSQ has an 'insert' that is implemented directly (not in terms of 'alter'), perhaps there is a reason to do the same in these other cases, but I let that up to you.

I have added some simple tests as well. Let me know if you want more (or add them yourself :-)).

meiersi commented 9 years ago

Hi Arie,

thanks for the proposal. I'd like to have a proper ok at it by myself before it gets merged. The takeMins function sounds a bit like a trivial function composition of a to ascPrioList and take. I think we shouldn't add such trivial compositions to an API to avoid combinatorial explosions in the number of functions to be exposed.

I'm currently on holidays. Will have a proper look at it when I'm back next Monday.

Best regards and thanks in advance, Simon

Arie notifications@github.com schrieb am Do., 16. Juli 2015 10:05 PM:

OK, I have done that.

The 'takeMin' function is easy to give in terms of the existing 'minView'.

The 'insertWith' for IntPSQ and HashPSQ I have implemented in terms of the 'alter' function. I hope this is OK; it should give at least the right complexity. Because OrdPSQ has an 'insert' that is implemented directly (not in terms of 'alter'), perhaps there is a reason to do the same in these other cases, but I let that up to you.

I have added some simple tests as well. Let me know if you want more (or add them yourself :-)).

— Reply to this email directly or view it on GitHub https://github.com/bttr/psqueues/pull/5#issuecomment-122072694.

ariep commented 9 years ago

Hi Simon,

[...] The takeMins function sounds a bit like a trivial function composition of a to ascPrioList and take.

Right, but there is currently no toAscPrioList. If there were, indeed I wouldn't have suggested takeMin.

I think we shouldn't add such trivial compositions to an API to avoid combinatorial explosions in the number of functions to be exposed. I'm currently on holidays. Will have a proper look at it when I'm back next Monday.

Sure, please do as seems best. I also don't mind if you include takeMin now and drop it later once there is a toAscPrioList (but I imagine you perhaps want a non-decreasing API).

Kind regards,

Arie

treeowl commented 9 years ago

Instead of takeMin, I'd recommend a version of foldr that works in priority order. I can work that up this week.

meiersi commented 9 years ago

Sounds good @ foldrPrio

David Feuer notifications@github.com schrieb am Fr., 23. Okt. 2015 3:28 AM:

Instead of takeMin, I'd recommend a version of foldr that works in priority order. I can work that up this week.

— Reply to this email directly or view it on GitHub https://github.com/bttr/psqueues/pull/5#issuecomment-150401947.