Document the laws of the -WithIndex classes - #58
Conversation
Add the compatibility laws (fmap = imap . const, foldMap = ifoldMap . const, traverse = itraverse . const), the agreement between imap/ifoldMap and their itraverse-derived defaults (imapDefault/ifoldMapDefault), and an index uniqueness/stability note to the FunctorWithIndex, FoldableWithIndex, and TraversableWithIndex class docstrings. Resolves ekmett/lens#745.
RyanGlScott
left a comment
RyanGlScott
left a comment
There was a problem hiding this comment.
LGTM, aside from one minor thing.
| - Document the laws of `FunctorWithIndex`, `FoldableWithIndex`, and | ||
| `TraversableWithIndex`: the `fmap`/`foldMap`/`traverse` compatibility laws, | ||
| agreement with `imapDefault`/`ifoldMapDefault`, and index uniqueness/stability. | ||
| Resolves <https://github.com/ekmett/lens/issues/745>. |
There was a problem hiding this comment.
Normally, I'd be fine with mentioning the issue being fixed, but it does feel a bit weird to mention a lens issue in a completely different library's changelog. I think we should just omit the "Resolves ekmett/lens#74" bit.
| -- | ||
| -- The index of an element is determined by its position in the structure, not | ||
| -- by the element's value, so 'imap' leaves the indices unchanged. The indices | ||
| -- visited within a single structure should be distinct, and are the same |
There was a problem hiding this comment.
Not true. There can be duplicate indices. What is a reason to require them to be distinct?
A simple example is multimap. More complex example is index maps (in lens or optics) which can produce indexed optics with the same indices.
So, I don't see a reason to require indices to be unique in the first place.
There was a problem hiding this comment.
I removed the uniqueness wording. The docstring now only states fmap f ≡ imap (const f) plus the cross-class index agreement, neither of which implies distinct indices.
3 participants
Resolves ekmett/lens#745.
The
FunctorWithIndex,FoldableWithIndex, andTraversableWithIndexclasses (whichlensandopticsre-export from here) only documented some of their laws. This adds, as Haddock-only changes toWithIndex.hs:fmap f ≡ imap (const f),foldMap f ≡ ifoldMap (const f),traverse f ≡ itraverse (const f);TraversableWithIndex'simap/ifoldMapmust agree withimapDefault/ifoldMapDefault;imap) and consistent across the three classes.One thing to confirm: ekmett raised wanting a uniqueness-of-indices law. I phrased it as indices "should be distinct" (SHOULD) since all standard instances satisfy it — happy to harden it to a MUST if you'd prefer.
Doc-only;
cabal build+cabal haddockof the package are clean (verified locally on GHC 9.14.1).