Skip to content

Document the behavior of fromListWith with non-associative and non-commutative operations #237

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

Conversation

Lysxia
Copy link
Contributor

@Lysxia Lysxia commented Jun 23, 2019

Also copy the improvements of #133 from Strict to Lazy.

On top of f newVal oldVal, we also need to explain that the elements are accumulated left-to-right. The fact that fromListWith (++) gives the elements in reverse order seems somewhat counter-intuitive to me, but it is what it is.

@emilypi
Copy link
Member

emilypi commented May 28, 2020

For reference, this is what's produced:

haddocks

This looks fine to me, unless there's a preference for fancydoc styles.

Copy link
Member

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

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

Cheers! This is very valuable documentation!

My suggestions below are optional, since this is already a large improvement on the status quo.

@sjakobi
Copy link
Member

sjakobi commented May 29, 2020

BTW, CI has been fixed in the meantime, so a rebase should take care of the red status here.

@Lysxia Lysxia force-pushed the doc-fromListWith-bis branch from 3c91418 to 37ea567 Compare May 29, 2020 13:33
@Lysxia
Copy link
Contributor Author

Lysxia commented May 29, 2020

Thanks for the reviews :)

@Lysxia Lysxia force-pushed the doc-fromListWith-bis branch 5 times, most recently from d88aaa6 to 54499b4 Compare May 29, 2020 14:10
@Lysxia
Copy link
Contributor Author

Lysxia commented May 29, 2020

I made the two existing examples more concrete. The last one is tricky to improve because it would be best illustrated with a non-associative operation, and a good instance is hard to come by.

Copy link
Member

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

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

Thanks, that's a great concrete example! :)

I think that the "gotcha" documented here is a bit too important to be hidden within the collapsible Examples section though. You could add a second heading to end the collapsible section, or alternatively just remove the collapsible section heading.

Apologies for the endless bikeshedding BTW! ;)

…mmutative operations

Also:

- Copy the improvements of haskell-unordered-containers#133 from Strict to Lazy
- Make the examples more concrete
@Lysxia Lysxia force-pushed the doc-fromListWith-bis branch from 54499b4 to b6e5c31 Compare May 30, 2020 14:49
@Lysxia
Copy link
Contributor Author

Lysxia commented May 30, 2020

Fair enough!

Copy link
Member

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

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

Thanks again, @Lysxia! :)

@sjakobi sjakobi merged commit 1757e89 into haskell-unordered-containers:master May 31, 2020
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.

3 participants