bloom-lang / bud

Prototype Bud runtime (Bloom Under Development)
http://bloom-lang.net
Other
854 stars 60 forks source link

Added lset group_count operator #328

Closed atancoder closed 6 years ago

jhellerstein commented 6 years ago

Hi -- this is great! I'm a concerned that you're assuming the lset holds Array objects. Isn't it possible to have an lset of, say, integers? Seems like the current code would break in that circumstance yes?

More generally, important to have the unit test work through obvious failure exceptions and exception handling.

atancoder commented 6 years ago

You're absolutely right. I made some changes to restrict lsets to only contain arrays if using the group_count operator. I additionally added a check to verify a column index you want to group by is actually a valid index in the array. I wrote the appropriate unit tests to check those cases.

Let me know if you were thinking of a different way to fix the issue.