Skip to content

Fix haddocks of Data.IntMap.compose #1143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 13, 2025
Merged

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
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