Skip to content

Conversation

@tomsmeding
Copy link
Contributor

The words "former" and "latter" in the documentation of compose on IntMap were swapped. This PR puts them the right way round.

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

LGTM

@alexfmpe
Copy link
Contributor

Mind fixing this in Data.Map.compose as well?

@meooow25
Copy link
Contributor

The docs say

-- | Relate the keys of one map to the values of
-- the other, by using the values of the former as keys for lookups
-- in the latter.
...
compose :: IntMap c -> IntMap Int -> IntMap c

which looks correct to me:

-- | Relate the keys of one map (IntMap Int) to the values of
-- the other (IntMap c), by using the values of the former (IntMap Int) as keys for lookups
-- in the latter (IntMap c).

@tomsmeding
Copy link
Contributor Author

Ah, my bad. You are both correct:

  1. The hadocks for Data.Map could to be improved too
  2. The issue is not actually with former/latter, but with the fact that the "former" is the second argument to compose and the "latter" is the first argument to compose, which confused me, @Mikolaj and @alexfmpe -- @meooow25 was the first to notice.

I've force-pushed a new suggestion. Thanks for reading critically!

@Mikolaj
Copy link
Member

Mikolaj commented May 12, 2025

Aha! "Relate" as in "map", as opposed to as in "identify". I think this proves the text needs to be rephrased totally.

@tomsmeding
Copy link
Contributor Author

I think "relate" can be justified, because a Map (and an IntMap) is, after all, a relation.

@alexfmpe
Copy link
Contributor

Maybe "link" ?

@tomsmeding
Copy link
Contributor Author

Perhaps:

Given maps bc and ab, build a new map that relates keys of ab to values of bc, by using the values of ab as keys for lookups in bc.

This still uses "relates", but uses it in a way that makes it fairly unambiguous what it means, I think. @Mikolaj?

@Mikolaj
Copy link
Member

Mikolaj commented May 13, 2025

Yes, this helps.

Copy link
Contributor

@meooow25 meooow25 left a comment

Choose a reason for hiding this comment

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

Thanks, this does seem like an improvement.

@meooow25 meooow25 merged commit 2d1c25d into haskell:master May 13, 2025
13 checks passed
sjakobi pushed a commit to haskell-unordered-containers/unordered-containers that referenced this pull request Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants