search

typelevel / algebra

Experimental project to lay out basic algebra type classes
https://typelevel.org/algebra/
Other
378 stars 69 forks source link

De Morgan algebra #224

Closed vquintin closed 5 years ago

vquintin commented 5 years ago

The motivation of this PR is to fix the invalid Heyting instance for Trilean in spire. Trilean is a DeMorgan, not a Heyting (it does not satisfy the law of non contradiction).

codecov-io commented 5 years ago

Codecov Report

Merging #224 into master will increase coverage by 0.11%. The diff coverage is 79.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #224      +/-   ##
==========================================
+ Coverage    76.4%   76.52%   +0.11%     
==========================================
  Files          42       45       +3     
  Lines         958     1001      +43     
  Branches       45       50       +5     
==========================================
+ Hits          732      766      +34     
- Misses        226      235       +9
Impacted Files Coverage Δ
core/src/main/scala/algebra/lattice/Heyting.scala 14.28% <ø> (ø) :arrow_up:
...aws/src/main/scala/algebra/laws/DeMorganLaws.scala 100% <100%> (ø)
core/src/main/scala/algebra/lattice/Logic.scala 50% <50%> (ø)
core/src/main/scala/algebra/lattice/DeMorgan.scala 88.88% <88.88%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 88bf270...749554b. Read the comment docs.

larsrh commented 5 years ago

This would be a binary-breaking change and requires a 1.x release.

Cats team, where are we with the incorporation of algebra into cats? @kailuowang @lukajcb

kailuowang commented 5 years ago

@larsrh sorry for the late reply. I think we are all on board in terms of moving it in. If anyone has time to make the PR I think we can make it happen in the next release, or the one after.

larsrh commented 5 years ago

@kailuowang OK, so I'd say we merge this in here regardless. Porting to Cats can happen at a later time.

LukaJCB commented 5 years ago

In terms of merging Cats, I think we still haven't decided how exactly we want to do that, if we want to have a new module vs. merging it into cats-core or cats-kernel. Though maybe it is best to just have a new module and then atleast have it in the same repo.

larsrh commented 5 years ago

Blocked by #225. If we want this to be binary compatible, we should enforce it.

larsrh commented 5 years ago

@vquintin I've merged in #224 and pushed to your branch to enable MiMa checking.

larsrh commented 5 years ago

@johnynek Are we good to merge this?

larsrh commented 5 years ago

1.0.2 release containing this change and 2.13.0-RC1 compatibility forthcoming.