Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add .sum method on Foldables using scala.Numeric #3643

Closed
kubukoz opened this issue Oct 21, 2020 · 7 comments · Fixed by #3823
Closed

Add .sum method on Foldables using scala.Numeric #3643

kubukoz opened this issue Oct 21, 2020 · 7 comments · Fixed by #3823

Comments

@kubukoz
Copy link
Member

kubukoz commented Oct 21, 2020

It would be good to have to enable summing NonEmptyLists like standard collections, similarly with other structures.

@kubukoz
Copy link
Member Author

kubukoz commented Oct 21, 2020

cc @hmemcpy

@hmemcpy
Copy link

hmemcpy commented Oct 21, 2020

Thanks! Yeah, I migrated some code from List to NonEmptyList and had to scratch my head for a second about .sum being an extension for SemigroupK. Replaced with .reduce and carried on, but it would be good to have .sum on a NEL that works as List.sum

@kubukoz
Copy link
Member Author

kubukoz commented Oct 21, 2020

Oh no, I didn't realise it was SemigroupK syntax... we'll need to find another name to avoid conflict, then.

@kubukoz
Copy link
Member Author

kubukoz commented Oct 21, 2020

I would suggest addAll.

And now that I think about it, we should have product too, but that's definitely overloaded as well from Semigroupal. So I'm thinking addAll / multiplyAll or sumAll / productAll.

@typelevel typelevel deleted a comment from Lealea9 Oct 22, 2020
@LukaJCB
Copy link
Member

LukaJCB commented Oct 22, 2020

Deleted a comment that was spam

@johnynek
Copy link
Contributor

I would really rather we not use scala.Numeric. I think we should instead be using Ring from algebra, which we have talked about merging into this repo as a separate subproject.

@GrigoriiBerezin
Copy link

Maybe we can use Semigroup in case the algebra is not merged into this repo yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants