Skip to content
This repository has been archived by the owner on Feb 8, 2022. It is now read-only.

Code gen for tuple instances #82

Merged
merged 5 commits into from
Oct 4, 2015
Merged

Conversation

adelbertc
Copy link
Contributor

No description provided.

@non
Copy link
Contributor

non commented Sep 17, 2015

This looks great.

Could you add support for (at least) PartialOrder, Semilattice, EuclideanRing, and Field?

In theory we could support all the other variants (e.g. the commutative versions, Bands, bounded lattices) but I'm willing to wait and see if anyone asks for those.

implicit val band = new Band[(Int, Int)] {
def combine(a: (Int, Int), b: (Int, Int)) = (a._1, b._2)
}
checkAll("(Int, Int) Band", GroupLaws[(Int, Int)].band)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you delete this band test? If nothing else it was exercising GroupLaws.band.

@johnynek
Copy link
Contributor

@non you steered him wrong on Field, my friend.

@johnynek
Copy link
Contributor

Of course, it would be cool to have a Galois fields for tuples of Boolean. :) (which is the one exception that pops to mind for standard scala types, but it does not compose this way).

@adelbertc
Copy link
Contributor Author

@johnynek Thanks for your feedback, will address them soon! I'm out of town for the weekend so most likely won't be able to do anything until next week.

- def negate(x: ${`(A..N)`}): ${`(A..N)`} = ${unaryTuple("negate")}
- def one: ${`(A..N)`} = ${nullaryTuple("one")}
- def plus(x: ${`(A..N)`}, y: ${`(A..N)`}): ${`(A..N)`} = ${binTuple("plus")}
- def quot(x: ${`(A..N)`}, y: ${`(A..N)`}): ${`(A..N)`} = ${binTuple("quot")}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm skeptical of this one as well. Can you add a test that checks that (Int, Int) forms a EuclideanRing? I don't know as much about the theory of euclidean rings, so I don't know off hand the answer.

@non
Copy link
Contributor

non commented Sep 25, 2015

👍

@non
Copy link
Contributor

non commented Sep 25, 2015

@johnynek I encouraged @adelbertc to add the euclidean ring instance (as well as the erroneous field instance). I'm pretty sure it's OK (although I haven't written a proof yet) but if you are nervous I would be willing to remove it (and obviously apologize to @adelbertc for suggesting the work).

@johnynek
Copy link
Contributor

johnynek commented Oct 4, 2015

Yeah, I guess it works for a Euclidean Ring, but I noticed that we don't actually have a test for Euclidean Rings. It is currently a TODO and actually just checking the Ring laws.

+1 from me. Added #85

johnynek added a commit that referenced this pull request Oct 4, 2015
@johnynek johnynek merged commit b7df598 into typelevel:master Oct 4, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants