-
Notifications
You must be signed in to change notification settings - Fork 345
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
Rewrote set tutorial from scratch. #1596
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, I like the things that you've added a lot, this is vastly clearer for a beginner and should be of great use to people starting out.
It might still be nice to include some of the information you've disposed of. For example, it would be nice to explain that Sets are pure functional (and that you can expect that from the type signatures), and a bit about what the provided operations are. Also it would be nice to explain the distinction between the int
type and the Int
module and why you can use the one but not the other.
site/learn/tutorials/set.md
Outdated
The type signature for `empty` says that it simply returns `t`, i.e. an instance | ||
of our set, without requiring any parameters at all. By intuition, you might | ||
guess that the only reasonable set that a library function could return when | ||
given zero parameters is the empty list. And the fact that the function is named |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should read is the empty set
, not, is the empty list
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, will fix.
This is intended to address the issue mentioned in ocaml/ocaml.org#598 This version of the tutorial now demonstrates how to use Set with arbitrary types. It also provides a demonstration on how to reason about the behavior of functions based on their type signatures.
Thanks for the feedback. I think I've addressed them in this latest commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more small comment from me, but overall I think this looks good. It would be nice to get another opinion of course.
`Set.Make(string)`. The reason behind this is explained in the | ||
"Technical Details" section at the bottom. | ||
|
||
Doing this in the OCaml's top level will yield a lot of output: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're not displaying all the output, you may want to add ellipses here or some such.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand this comment.
The original MD file just contains the one line you'd type into OCaml's top level:
To make a set of strings:
```ocamltop
module SS = Set.Make(String);;
```
But the output on https://ocaml.org/learn/tutorials/set.html contains not just the line you'd enter in, but also the output produced by OCaml's top level (I'm assuming whatever static site generator you're using is responsible for doing this). So my MD is doing something similar: I'm putting just the line that the user would enter in, and I'm expecting the static site generator to produce a huge amount of input.
The "Doing this in the OCaml's top level will yield a lot of output:" line I put there was intended to prepare new users to not panic in response to the huge block of text they're about to see (my reaction to seeing that initial huge block was panic).
@@ -3,74 +3,252 @@ | |||
|
|||
# Set | |||
|
|||
## Module Set | |||
To make a set of strings: | |||
`Set` is a functor, which means that it is a module that is parameterized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tutorial needs some introduction: it is reachable from the Data structures
section of the tutorials. Thus when landing on the page the reader is probably expecting to learn about Set
as a data structure. Starting with "Set
is a functor" is thus doubly confusing: "What is a functor? Wasn't I promised a tutorial about a data structure?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the right thing then is to split this into two tutorials, one about sets-as-functors and another about how to use sets for their own sake?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The introduction should start by explaining what is a set from OCaml point of view : a collection of ordered elements. Then it would make sense to the reader that the Set.Make
functor is building a set module from a module of ordered elements.
receives a set as a parameter, and returns an integer as a result. Again, | ||
skimming through the list of functions in the module, we see there is a | ||
function which matches this signature: `cardinal : t -> int`. If you're | ||
not familiar with the word "cardinal", you can look it up on Wikipedia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that sending the reader to a Wikipedia page is a good idea in general: either trust the reader to know the term or explain it. This particularly true for the mathematical section of Wikipedia which tends to be quite technical. Typically, the cardinal page of wikipedia cites the axiom of choice in its introduction.
the type signature of functions are often the most convenient form of documentation | ||
on how to use those functions. | ||
|
||
## Creating a Set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of the section doesn't match the content. This section is describing how to analyze an unknown module, not how to create a set. I would propose to move this section later, and simply start by creating a set in the tutorial.
|
||
For example, the `add` function has the signature `elt -> t -> t`, which means | ||
that it expects an element (a String), and a set of strings, and will return to you | ||
a set of strings. As you gain more experience in OCaml and other function languages, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function languages ⇒ functional languages
I am not that fond of meta comments that discourse about the expected proficiency of the reader.
which you can assign the name `SS` (short for "String Set"). Note: Be sure to | ||
pay attention to the case; you need to type `Set.Make(String)` and not | ||
`Set.Make(string)`. The reason behind this is explained in the | ||
"Technical Details" section at the bottom. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delaying a simple explanation "String
is a module" (?) to a technical section seems unnecessary?
This can be written as: | ||
It looks like our theories were correct! | ||
|
||
## Sets of With Custom Comparators |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sets with custom comparators?
After a quick glance, there are good ideas in the rewritten text. Nevertheless, I see two issues. First, the text is lacking an introduction. Second, there are two tutorials fighting each other currently: a "how to use a functor" tutorial and a set tutorial. Currently, the |
Hi @Octachron I've skimmed through your various suggestions and I will attempt to address them in another revision of the pull request, but I'd like to address your "overarching feedback" here:
This rewrite was done in response to ocaml/ocaml.org#598 where someone pointed out that the original tutorial gave an example of creating a set of strings via I agree with their assessment, because as a new OCaml user, I was also surprised why one worked and the other didn't. This might be especially likely for people with a Java background or similar, where the type for String is called After performing my own research on why This tutorial is primarily intended to be a tutorial on the using sets in OCaml with a target audience of people familiar with programming in general, but with zero OCaml experience (e.g. someone who has used set-like data structures in Java or other similar languages). I did not intend to focus specifically on how to use functors in the general case (to be honest, I don't know how to use functors in general in OCaml), but I do feel there is a minimum you need to know about functors in OCaml to actually use the Set module. If there are more words spent on the topic of functors than on the topic of sets, it's probably because I expect most readers to know basically what a set is and how it works in other languages, but to be utterly confounded by this concept of "functor" which is likely to be totally alien to them. Especially since I don't expect the reader to have a good understanding of "modules" in the first place when they first stumble onto this page (I did not have a good understanding of modules when I first encountered this page). Does that make sense? Edit to add one more thought: One possibility is to tell the reader to first read the tutorials on modules and functors before reading this tutorial on sets, but I feel like that would be too discouraging to a new OCaml user. So I think it makes sense to do some minimal introduction to functors here, rather than ask the reader to learn about functors first. |
I agree that the tutorial should not assume too much familiarity with functors. And that it is necessary to have good functor tutorial. However, this is the |
that it expects an element (a String), and a set of strings, and will return to you | ||
a set of strings. As you gain more experience in OCaml and other function languages, | ||
the type signature of functions are often the most convenient form of documentation | ||
on how to use those functions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skipped that part before, but no types
are not the most convenient form of documentation. It is the most basic form of documentation.
We might be at an impasse here. What's the conflict resolution strategy used for this repo if some people like the change and some people don't like it? Do we vote, or does "one dislike" veto the change, or what? I don't want to bother addressing the minor issues (e.g. the "function language" -> "functional language" typo) if the PR is likely to be rejected in the end. In case it'll sway your vote, I claim that even if this tutorial isn't the ideal tutorial you have in mind, it better serves the goal of having a new OCaml programmer become comfortable with using Set than the previous tutorial, and so it'd be an overall positive if merged in. That's just my (probably biased) opinion, though. |
I am not sure why you think that we are at an impasse? What is the issue with rerouting the functor related explanations to the functor tutorial? |
We could split this in two perhaps? Explain functors in one part, and the data structure itself in another, and advise readers to skip the functor section if and only if they're comfortable with functors? |
(BTW, this is a very clear explanation of functors for the new user who thinks they're just interested in sets but needs to understand functors to make use of sets.) |
I re-read the tutorial I wrote, and I actually don't think I spend very much time talking about functors at all. As far as I can see, this is the only explanation I give on functors:
Every other sentence in the tutorial is about Set. The word "functor" appears a total of 4 times in the tutorial, including the sentence I quoted above which gives a one sentence definition of the term. The other 3 uses are always "the I could change it to "module", as in "The So I guess my issue is that it's not clear to me what in the tutorial seems to be "about functors" that should be split off. |
Hm, I think that I see what you mean. From my point of view, when you say
you are discarding the set data structure and talking about how to make sense of the result of a generic functor. Similarly, the sections Maybe another editing direction will be to start describing the basic set functions before diving in type exploration mode to discover more functions for creating or updating sets. I think that grounding a little more those sections in the data structure being described might remove my impression of reading a generic functor tutorial. |
I think I better understand your point of view now. Let me think about this some more and I'll probably submit another revision. |
@NebuPookins rewrote the set tutorial for [V2](https://github.com/ocaml/v2.ocaml.org) in 2021. The PR was neither merged nor rejected: ocaml/v2.ocaml.org#1596
@NebuPookins rewrote the set tutorial for [V2](https://github.com/ocaml/v2.ocaml.org) in 2021. The PR was neither merged nor rejected: ocaml/v2.ocaml.org#1596
@NebuPookins rewrote the set tutorial for [V2](https://github.com/ocaml/v2.ocaml.org) in 2021. The PR was neither merged nor rejected: ocaml/v2.ocaml.org#1596
@NebuPookins rewrote the set tutorial for [V2](https://github.com/ocaml/v2.ocaml.org) in 2021. The PR was neither merged nor rejected: ocaml/v2.ocaml.org#1596
* Import Rewrote Set Tutorial from V2 PR @NebuPookins rewrote the set tutorial for [V2](https://github.com/ocaml/v2.ocaml.org) in 2021. The PR was neither merged nor rejected: ocaml/v2.ocaml.org#1596 * Apply suggestions from code review Co-authored-by: Christine Rose <christinerose@users.noreply.github.com> * Minor Fixes As pointed by @Octachron * line editing * shorten a lot and present common operations instead of explaining functors * Review edits --------- Co-authored-by: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com> Co-authored-by: Christine Rose <christinerose@users.noreply.github.com> Co-authored-by: Sabine Schmaltz <sabineschmaltz@gmail.com>
Issue Description
Fixes ocaml/ocaml.org#598
Changes Made
This version of the tutorial now demonstrates how to use Set with
arbitrary types. It also provides a demonstration on how to reason
about the behavior of functions based on their type signatures.