Improve Data.Set module docs. #502
Conversation
[ci skip]
Data/Set.hs
Outdated
| -- = Sets | ||
| -- | ||
| -- The @'Set' e@ type represents a set of /unique/, /ordered/ elements of type | ||
| -- @e@. |
There was a problem hiding this comment.
What are ordered elements? What is a unique element? I think perhaps
-- The @'Set' e@ type represents a set of elements of type @e@. Most operations
-- require that @e@ be an instance of the 'Ord' class.
Data/Set.hs
Outdated
| -- first argument are always preferred to the second, for example in | ||
| -- 'union' or 'insert'. Of course, left-biasing can only be observed | ||
| -- when equality is an equivalence relation instead of structural | ||
| -- equality. |
There was a problem hiding this comment.
See my note on Map. Also, rather than "left-biasing", how about "this bias"?
Data/Set.hs
Outdated
| -- | ||
| -- Here are some examples that illustrate the property: | ||
| -- | ||
| -- > delete undefined s == undefined |
There was a problem hiding this comment.
This demonstrates nothing. How about
-- > insert undefined xs == undefined
-- > singleton undefined = undefinedThere was a problem hiding this comment.
I've removed the strictness section (as I did in the map docs). All the information here is in the first section that discusses the strictness of the element type.
|
I don't see the update. |
|
Oops, accidentally sent the review comments before pushing the commit. I thought I had "started a review" which I was going to finish after the commit went up. I'll have it up shortly, about to walk to work. |
[ci skip]
|
Changes pushed, Haddocks haven't been pushed again because its a pain to get both changes built simultaneously because they're in different branches. |
Data/Set.hs
Outdated
| -- = Finite Sets | ||
| -- | ||
| -- The @'Set' e@ type represents a set of elements of type @e@. A 'Set' is | ||
| -- strict in the element type and most operations require that @e@ be an |
There was a problem hiding this comment.
Surely you mean strict in its elements, not their type. I think you should move the strictness to a new sentence after the one about Ord.
There was a problem hiding this comment.
Oops, yup that what I meant.
There was a problem hiding this comment.
Maybe this should just be a bullet list, it may read better since both the sentences are so short. WDYT?
[ci skip]
|
I don't think bullets will help.
…On Jan 22, 2018 10:54 AM, "Matt Renaud" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In Data/Set.hs
<#502 (comment)>:
> @@ -13,14 +13,38 @@
-- Maintainer : ***@***.***
-- Portability : portable
--
--- An efficient implementation of sets.
+--
+-- = Finite Sets
+--
+-- The @'Set' e@ type represents a set of elements of type @***@***.*** A 'Set' is
+-- strict in the element type and most operations require that @e@ be an
Maybe this should just be a bullet list, it may read better since both the
sentences are so short. WDYT?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#502 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABzi_eZtHBfJHAv7J8Keyn_ekxAhkaXVks5tNK8vgaJpZM4RmKLW>
.
|
|
How about: |
|
Technically, I suppose that's true. But it's much more intuitively helpful,
I believe, to think of Set as a strict data structure.
…On Jan 22, 2018 11:01 AM, "Matt Renaud" ***@***.***> wrote:
How about: "Most operations require that @e <https://github.com/e>@ be an
instance of the 'Ord' class, and all operations are strict in the element."
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#502 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABzi_WP60sVHI6hUbrvxG54qdYCvmkVOks5tNLDogaJpZM4RmKLW>
.
|
|
Gotcha, I'll leave it as is. Are there any other changes you'd like made or can this be merged? |
This improves the module level docs for Data.Set following the form of the recent improvements to Data.Sequence. This is for the most part a re-organization of the existing content with a few minor changes so should hopefully be uncontroversial.
Rendered docs can be found here.
Partially addresses #495.