Skip to content
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

docs: refresh Map tutorial #2045

Merged
merged 13 commits into from
Feb 21, 2024
Merged

docs: refresh Map tutorial #2045

merged 13 commits into from
Feb 21, 2024

Conversation

leostera
Copy link
Contributor

Hello folks! Here's a refresh on the Map tutorial. I followed the style of the new Set tutorial to make the structure easy to recognize.

As always, happy to change anything :)

Comment on lines 45 to 46
let int_map : int StringMap.t = StringMap.empty;;
let float_map : float StringMap.t = StringMap.empty;;
Copy link
Collaborator

Choose a reason for hiding this comment

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

In these code examples for the toplevel, we've been starting them with a # and putting the expected result in the last line. Shall I go through and do that?

- : unit = ()
The behavior of `StringMap.union` can be tuned based on the conflict function we pass in.

For example, we can always pick the value from the first map, or choose to sum both values together (which would only work for values of type `int`), or we can even choose to ignore a value entirely if it is present in both maps.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since there are three choices, which makes for a long, slightly confusing sentence, what do you think of making these a bullet list?

- : int = 42
### Filtering a Map

To filter a map, we can use the `filter` function. `filter` takes a predicate to filter entries by and a map. It returns a new map with only the entries that passed the filter.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
To filter a map, we can use the `filter` function. `filter` takes a predicate to filter entries by and a map. It returns a new map with only the entries that passed the filter.
To filter a map, we can use the `filter` function. `filter` filters entries by taking a predicate and a map. It returns a new map with only the entries that passed the filter.

Does this make things more clear or is it now incorrect?

;;
```

Note that our module has a `type t` and also a `compare` function. Now we can call the **Map.Make** function on it, to get a map for non-negative numbers:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Note that our module has a `type t` and also a `compare` function. Now we can call the **Map.Make** function on it, to get a map for non-negative numbers:
Note that our module has a `type t` and also a `compare` function. Now we can call the **Map.Make** function to get a map for non-negative numbers:

Copy link
Collaborator

@christinerose christinerose left a comment

Choose a reason for hiding this comment

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

Made some grammar/formatting changes. A few suggestions pending as well as a couple of questions, including one about formatting code examples.

Copy link
Collaborator

@cuihtlauac cuihtlauac left a comment

Choose a reason for hiding this comment

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

I've prepared a patch with my edits

@cuihtlauac
Copy link
Collaborator

@cuihtlauac
Copy link
Collaborator

Also, checks are failing because the formatting was broken upstream. Please rebase.

@sabine
Copy link
Collaborator

sabine commented Feb 19, 2024

0001-Formatting-and-editing.patch.gz

do you by any chance have that as a branch somewhere?

@cuihtlauac
Copy link
Collaborator

do you by any chance have that as a branch somewhere?

It's there:
https://github.com/ocaml/ocaml.org/tree/doc-maps

Cuihtlauac ALVARADO added 2 commits February 20, 2024 13:57
@cuihtlauac
Copy link
Collaborator

I have two more commits there: https://github.com/ocaml/ocaml.org/tree/doc-maps

Comment on lines 73 to 78
# let lucky_numbers = StringMap.of_list [
("leostera", 2112);
("charstring88", 88);
("divagnz", 13);
];;
val lucky_numbers : int StringMap.t = <abstr>
Copy link
Collaborator

@christinerose christinerose Feb 21, 2024

Choose a reason for hiding this comment

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

I get Error: Unbound value StringMap.of_list here

Copy link
Collaborator

@cuihtlauac cuihtlauac Feb 21, 2024

Choose a reason for hiding this comment

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

That's an OCaml version issue. of_list was added in OCaml 5.1. We can have this to make compatible with 4.14

let lucky_numbers = StringMap.of_seq @@ List.to_seq [

BTW. Should it be a requirement for all tutorials? Be compatible with 4.14 ? Since it is LTS it probably make sense.

We can have 5.x related stuff as remarks/notes

@christinerose christinerose merged commit b83b22c into ocaml:main Feb 21, 2024
2 of 3 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