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

Added doc for more string-dict methods #27

Open
wants to merge 2 commits into
base: horizon
Choose a base branch
from

Conversation

ds26gte
Copy link

@ds26gte ds26gte commented Nov 6, 2017

This addresses brownplt/pyret-lang#1245. It adds documentation for the immutable string-dict methods {merge, keys-list, map-keys, fold-keys, each-key} and the mutable-string-dict methogs {merge-now, keys-list-now, map-keys-now, fold-keys-now, each-key-now}.

fold-keys each-key; and for mutable-string-dict methods: merge-now
keys-list-now map-keys-now fold-keys-now each-key-now.
Copy link
Member

@blerner blerner left a comment

Choose a reason for hiding this comment

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

Various tightenings and improvements, and one important correction.

]

Returns a new immutable string-dict that has the keys of both
the original string-dict and the @pyret{other} string-dict. Choose a key's
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't use the verb "choose" here, since that sounds to me like the programmer has a choice. I'd reword this to "In cases where both input dictionaries have the same key, the resulting value comes from the @pyret{other} string-dict."

sd3 is [string-dict: "a", 10, "b", 6, "c", 4]
sd4 = sd2.merge(sd1)
sd4 is [string-dict: "a", 5, "b", 6, "c", 4]
sd2.merge(sd1).merge(sd1) is sd2.merge(sd1)
Copy link
Member

Choose a reason for hiding this comment

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

# merging the same dictionary twice has no additional effect

sd3 = sd1.merge(sd2)
sd3 is [string-dict: "a", 10, "b", 6, "c", 4]
sd4 = sd2.merge(sd1)
sd4 is [string-dict: "a", 5, "b", 6, "c", 4]
Copy link
Member

Choose a reason for hiding this comment

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

# NOTE: sd3 is-not sd4

#:return (L-of S)
]

Returns the list of keys in the immutable string-dict, in some order.
Copy link
Member

Choose a reason for hiding this comment

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

"in some unspecified order"

@examples{
check:
sd2 = [string-dict: "a", 10, "b", 6]
sd2.keys-list() is [list: "a", "b"]
Copy link
Member

Choose a reason for hiding this comment

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

If the order is unspecified, then this test might fail. Either add a .sort() or make it into a set or something...

]

Modifies the mutable string-dict to include the keys from the @pyret{other}
mutable-string-dict. Choose a key's value to be the one it has in the @pyret{other}
Copy link
Member

Choose a reason for hiding this comment

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

ditto

#:return (L-of S)
]

Returns the list of keys in the mutable string-dict, in some order.
Copy link
Member

Choose a reason for hiding this comment

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

ditto

#:return (L-of "b")
]

Modify the mutable string-dict so that each key is now
Copy link
Member

Choose a reason for hiding this comment

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

No -- this returns a List<b>, and the description sounds like it returns a MSD.


@examples{
fun one-of(ans, elts):
is-some(for find(elt from elts):
Copy link
Member

Choose a reason for hiding this comment

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

ditto

#:return No
]

Calls the function @pyret{f} on each key in the mutable string-dict,
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Author

Choose a reason for hiding this comment

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

Thanks -- made your suggested changes. When having the description of each-key refer to map-keys, I tried using @pyret-id{map-keys} rather than @pyret{map-keys} to make it a hyperlink, but it doesn't seem to work. Is there a Pyret scribble quickref that will show how to ensure that @pyret-id{...} does link.

Copy link
Member

Choose a reason for hiding this comment

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

Use @pyret-method["StringDict"]{each-key} to link to the each-key method of the StringDict data type.

Copy link
Member

@blerner blerner left a comment

Choose a reason for hiding this comment

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

Minor tweaks, and using pyret-method where needed, and then this is fine.

check:
sd2 = [string-dict: "a", 10, "b", 6]
sd2.map-keys(lam(x): string-append(x, x) end) is%(one-of)
sd2.map-keys(lam(x): string-append(x, x) end) is%(lam(lft, rt): rt.member(lft) end)
Copy link
Member

Choose a reason for hiding this comment

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

This is complicated-looking. Why not use .sort() like you did on line 273 above for keys-list?

check:
msd2 = [mutable-string-dict: "a", 10, "b", 6]
msd2.map-keys-now(lam(x): string-append(x, x) end) is%(one-of)
msd2.map-keys-now(lam(x): string-append(x, x) end) is%(lam(lft, rt): rt.member(lft) end)
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@schanzer
Copy link

@ds26gte this PR seems awfully close to being mergeable. Any chance you can put an hour towards getting it across the finish line?

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