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

Code gen for tuple instances - #82

Merged
johnynek merged 5 commits into
typelevel:masterfrom
adelbertc:tuple-codegen
Oct 4, 2015
Merged

johnynek merged 5 commits into
typelevel:masterfrom
adelbertc:tuple-codegen

Conversation

@adelbertc

Copy link
Copy Markdown
Contributor

No description provided.

@non

non commented Sep 17, 2015

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
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
Copy Markdown
Contributor

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

@johnynek

Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread project/Boilerplate.scala

Copy link
Copy Markdown
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

non commented Sep 25, 2015

Copy link
Copy Markdown
Contributor

👍

@non

non commented Sep 25, 2015

Copy link
Copy Markdown
Contributor

@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

johnynek commented Oct 4, 2015

Copy link
Copy Markdown
Contributor

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