WIP:adding alegebraic typeclasses #1551

caente · 2017-12-01T16:30:01Z

First stab at expanding the Algebra hierarchy.

EDIT: Removed Magma

caente · 2017-12-01T16:46:37Z

The failing tests are because the Semigroup.meta.append change, but again, I'm not sure what's going on at SemigroupSyntax

vmarquez · 2017-12-01T18:19:59Z

Does Group have any laws? I don't know if we need it... (non rhetorical, curious to see what people think). cc @TomasMikula @jdegoes ?

Edit: s/Group/Magma

TomasMikula · 2017-12-01T18:35:17Z

You mean does magma have any laws? It doesn't. I'm not terribly interested in having it.

…

On Dec 1, 2017 7:20 PM, "Vincent Marquez" ***@***.***> wrote: Does Group have any laws? I don't know if we need it... (non rhetorical, curious to see what people think). cc @TomasMikula <https://github.com/tomasmikula> @jdegoes <https://github.com/jdegoes> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1551 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAadHwideFmmDoS5n9rwtgHOcwwGwUdnks5s8ENSgaJpZM4QyliC> .

hrhino · 2017-12-01T18:35:07Z

base/shared/src/main/scala/scalaz/typeclass/Group.scala

+
+/* A Group is a Monoid with negation and inverse, such that
+ * a |+| inverse(a)  = monoid.empty
+ * inverse(a) |+| a  = monoid.empty


Aren't these Group laws? @vmarquez

these are the laws, as per @jdegoes comment #1526 (comment) we will implement laws later on

Yeah. I wrote this after not realizing that "Does Group have any laws?" was intended to mention "Magma" instead.

hrhino · 2017-12-01T18:36:10Z

base/shared/src/main/scala/scalaz/typeclass/GroupInstances.scala

+
+trait GroupInstances {
+
+  implicit val int: Group[Int] = new GroupClass[Int] {


This implies that Int's (semi)group/monoid instance is (0, +), not (1, *).

Is that intended?

good question, I have no answer for it, except that perhaps it is not time yet to add instances

We sure could use some tagging that @S11001001 worked on in Scalaz 7.

hrhino · 2017-12-01T18:36:49Z

base/shared/src/main/scala/scalaz/typeclass/Magma.scala

+ * */
+trait Magma[A] {
+  def append(a1: A, a2: => A): A
+}


Surely this is lawless. What good does it do us to have a Magma that's not a Semigroup?

hrhino · 2017-12-01T18:37:46Z

base/shared/src/main/scala/scalaz/typeclass/SemigroupClass.scala

@@ -1,6 +1,6 @@
 package scalaz
 package typeclass

-trait SemigroupClass[A] extends Semigroup[A]{
+trait SemigroupClass[A] extends Semigroup[A] with MagmaClass[A]{


These subclassings need to be reflected in BaseHierarchy.

even if we consider removing Magma, I added it to BH as well
3162600

vmarquez · 2017-12-01T18:38:25Z

Yeah, i'm of the opinion that if an algebraic structure doesn't have laws, let's ditch it.

caente · 2017-12-01T18:54:03Z

I completely agree that Magma seems unnecessary, and it adds awkwardness to the code...

caente · 2017-12-02T16:42:57Z

the only "law" that Magma must enforce is that the result of the binary operation belongs to the same set... which I would think is already given by the type system, will remove it.

the only "law" that Magma must enforce is that the result of the binary operation belongs to the same set... which I would think is already given by the type system

vmarquez · 2017-12-04T06:15:00Z

@caente is this a WIP? If so, can you label it/change the name? When you're ready for a review/merge, can you squash the commits too? thanks :)

caente · 2017-12-04T13:13:55Z

@vmarquez I was hopping to have smaller PRs, but this is the first time doing this kind of things, and I'm not used to the slow pace... name changed, and definitely will squash commits.

caente · 2017-12-04T13:14:53Z

I'm actually waiting for some feedback regarding Rings @vmarquez @jdegoes @hrhino @TomasMikula

caente · 2017-12-04T13:45:28Z

also @jdegoes mentioned Commutative as a typeclass, but it seems to me that it should "just" be a law? I'd still want to understand why are we deferring laws implementation? Is it lack of consensus?

hrhino

Looking better.

I think that adding tut examples (see the examples/ directory) would help to ensure that all instances are properly resolved, and (of course) provide useful documentation for these types.

@NeQuissimus has done some great work in other scalaz 8 PRs regarding the examples; you should check them out.

hrhino · 2017-12-04T13:44:53Z

base/shared/src/main/scala/scalaz/typeclass/AbelianGroupInstances.scala

+
+trait AbelianGroupInstances {
+
+  implicit val int: AbelianGroup[Int] = new AbelianGroupClass[Int] {


So if you have this here it's probably going to clash with GroupInstances#int if you ask for Group[Int]. You should (assuming we intend to provide a Group[Int] at all) keep this and remove the other.

yeah, I agree with your above comment, these instances belong to examples, will move them

hrhino · 2017-12-04T13:51:49Z

base/shared/src/main/scala/scalaz/BaseHierarchy.scala

+    implicit def groupMonoid[A](implicit A:Group[A]):Monoid[A] = A.monoid
+    implicit def abelianGroupGroup[A](implicit A:AbelianGroup[A]):Group[A] = A.group
+    implicit def ringAbelianGroup[A](implicit A:Ring[A]):AbelianGroup[A] = A.abelianGroup
+    implicit def ringMonoid[A](implicit A:Ring[A]):Monoid[A] = A.monoid


This worries me; a Ring contains two distinct Monoids, and this picks the "multiplicative" one. However, groupMonoid compose abelianGroupGroup compose ringAbelianGroup also produces a Monoid from a Ring. This will cause the somewhat counter-intuitive behavior where asking for Monoid[A] gives you the multiplicative one, but calling a method that asks for Group[A] and then internally uses Monoid[A] will find the additive one.

I think we should have a rule such as

newtype Mult a = Mult a instance Ring a => Monoid (Mult a) where ...

using the (as yet forthcoming) tag types.

hrhino · 2017-12-04T13:53:06Z

base/shared/src/main/scala/scalaz/typeclass/Semigroup.scala

@@ -1,6 +1,11 @@
 package scalaz
 package typeclass

+/**
+ * A Semigroup is a binary opertion such that:
+ * 1 - The result is in the same Set/Type


This seems a little extraneous, as a quick glance at the type of append tells you exactly this. I'd opt for terseness and say

A semigroup is a type equipped with an associative binary operation

Also, opertion is misspelled...

hrhino · 2017-12-04T13:56:33Z

I think the lack of laws has been due to a lack of bandwidth. I was under the impression that scalaprops had been decided on as the law checking framework. I'd like to go poke at that some this week, if I find time.

jdegoes · 2017-12-04T23:32:18Z

@caente We don't have machinery to encode laws just yet. We need a data structure such as:

final case class IsEqual[A](lhs: A, rhs: A)

Then all the "laws" can return data.

It's OK to have a type class that merely adds another law to an existing structure (e.g. Commutative).

I will review in more depth later.

edmundnoble · 2018-06-06T00:07:30Z

I'm going to try to revive this tomorrow at the scalaz summit.

adding Magma and Group

2ee6a1d

caente changed the title ~~adding Magma and Group~~ WIP: adding Magma and Group Dec 1, 2017

caente changed the title ~~WIP: adding Magma and Group~~ adding Magma and Group Dec 1, 2017

caente mentioned this pull request Dec 1, 2017

Scalaz 8 Roadmap #1526

Open

99 tasks

hrhino reviewed Dec 1, 2017

View reviewed changes

added Magma >> Semigroup and Monoid >> Group to BH

3162600

removed Magma

8347966

the only "law" that Magma must enforce is that the result of the binary operation belongs to the same set... which I would think is already given by the type system

caente changed the title ~~adding Magma and Group~~ adding Group Dec 2, 2017

caente added 2 commits December 3, 2017 14:03

adding AbelianGroup

6481587

adding Ring

94eaba1

caente changed the title ~~adding Group~~ WIP:adding alegebraic typeclasses Dec 4, 2017

hrhino requested changes Dec 4, 2017

View reviewed changes

hobwekiva added the scalaz8 label Apr 8, 2018

hobwekiva modified the milestone: 8.0.0 Apr 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP:adding alegebraic typeclasses #1551

WIP:adding alegebraic typeclasses #1551

caente commented Dec 1, 2017 •

edited

caente commented Dec 1, 2017

vmarquez commented Dec 1, 2017 •

edited

TomasMikula commented Dec 1, 2017 via email

hrhino Dec 1, 2017

caente Dec 1, 2017 •

edited

hrhino Dec 1, 2017

hrhino Dec 1, 2017

caente Dec 1, 2017

TomasMikula Dec 1, 2017

hrhino Dec 1, 2017

hrhino Dec 1, 2017

caente Dec 1, 2017 •

edited

vmarquez commented Dec 1, 2017

caente commented Dec 1, 2017

caente commented Dec 2, 2017

vmarquez commented Dec 4, 2017

caente commented Dec 4, 2017

caente commented Dec 4, 2017

caente commented Dec 4, 2017

hrhino left a comment

hrhino Dec 4, 2017

caente Dec 6, 2017

hrhino Dec 4, 2017

hrhino Dec 4, 2017

hrhino commented Dec 4, 2017

jdegoes commented Dec 4, 2017

edmundnoble commented Jun 6, 2018 •

edited


		trait GroupInstances {

		implicit val int: Group[Int] = new GroupClass[Int] {


		trait AbelianGroupInstances {

		implicit val int: AbelianGroup[Int] = new AbelianGroupClass[Int] {

WIP:adding alegebraic typeclasses #1551

Are you sure you want to change the base?

WIP:adding alegebraic typeclasses #1551

Conversation

caente commented Dec 1, 2017 • edited

caente commented Dec 1, 2017

vmarquez commented Dec 1, 2017 • edited

TomasMikula commented Dec 1, 2017 via email

Choose a reason for hiding this comment

caente Dec 1, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caente Dec 1, 2017 • edited

Choose a reason for hiding this comment

vmarquez commented Dec 1, 2017

caente commented Dec 1, 2017

caente commented Dec 2, 2017

vmarquez commented Dec 4, 2017

caente commented Dec 4, 2017

caente commented Dec 4, 2017

caente commented Dec 4, 2017

hrhino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hrhino commented Dec 4, 2017

jdegoes commented Dec 4, 2017

edmundnoble commented Jun 6, 2018 • edited

caente commented Dec 1, 2017 •

edited

vmarquez commented Dec 1, 2017 •

edited

caente Dec 1, 2017 •

edited

caente Dec 1, 2017 •

edited

edmundnoble commented Jun 6, 2018 •

edited