typelevel / typelevel/cats

Bifoldable Laws with non-commutative types

Open
#4,361 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Scala
Stars
5.5k
Forks
1.2k
Avg merge
2d 10h
Merged PRs (30d)
5

Description

I noticed that the existing bifoldable laws for Binested fail if you use Tuple2 instead of Either as the two-slot type constructor, and a non-commutative monoid for the innermost types. e.g. adding

checkAll(
  "Binested[Tuple2, ListWrapper, ListWrapper, *, *]",
  BitraverseTests[Binested[Tuple2, ListWrapper, ListWrapper, *, *]]
    .bitraverse[Option, Int, Int, String, String, String, String]
)

fails cats.tests.BinestedSuite.Binested[Tuple2, ListWrapper, ListWrapper, *, *]: bitraverse.bifoldRight consistent with bifoldMap, vs the existing one, which passes:

https://github.com/typelevel/cats/blob/b036fb47196d1a788db09eb2bc262071fa3cb928/tests/shared/src/test/scala/cats/tests/BinestedSuite.scala#L86-L90

Assuming the implementation is correct and the failure hasn't revealed a bug, it makes me wonder if there's a way to encode this in the types. Should the C: Monoid on BitraverseTests.bitraverse be C: CommutativeMonoid so that the law is more universal? If so, how far up the chain should that propagate? Ultimately it's used in BifoldableLaws.bifoldLeftConsistentWithBifoldMap and BifoldableLaws.bifoldRightConsistentWithBifoldMap.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Binested law check in tests/shared/src/test/scala/cats/tests/BinestedSuite.scala and the C bound in laws/src/main/scala/cats/laws/discipline/BitraverseTests.scala. Run the Tuple2/non-commutative example, then trace BifoldableLaws.bifoldLeftConsistentWithBifoldMap and bifoldRightConsistentWithBifoldMap. Done means the law's required algebraic typeclass and its propagation are agreed and consistently represented in the tests and laws.

Written by the indexing model from the issue text.

Assessment

Tech stack
scala
Domain
testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.