-
Notifications
You must be signed in to change notification settings - Fork 569
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
Lens docs rewrite #1444
Lens docs rewrite #1444
Conversation
I'd welcome reviews even if it fails CI - the failures will be down to the examples, and I'd rather not invest time fixing them unless there was interest in the main content. |
``` | ||
|
||
That seems pretty simple and fairly annoying to write, which is why you | ||
generally don't have to. | ||
Now in addition to almost free `Clone`s, we also have cheap incremental updates to the data itself. In the case of names, this isn't that important, but if the vector had `1_000_000_000` elements, we could still make changes in only *O(log(n))* time (in this case the difference between `1_000_000_000` and `30` - pretty big!). |
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 found this a little confusing - where does 30 come from?
I quickly checked for both the natural log and the base-10 log, and neither was 30 - maybe a typo?
ln(1E9) = 20.72
log(1E9) = 9.00
Otherwise, this is an excellent point! It's a huuuuge savings on even moderately sized collections.
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.
It was from log2, it was really meant to be illustrative only - in reality you probably do better by having 32 elements per node (or even more).
|
||
### Getting something from a collection | ||
Finally, there is a macro for constructing lenses on the fly. It allows you to lens into fields of a struct you don't control (so you can't derive `Lens` for it), it also allows lensing into tuples and tuple structs, and lastly it will create index lenses into slices. |
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.
Definitely good to bring this up. I think we need to demonstrate how this macro invocation is written, though. Maybe a contrived example on a standard library type?
7e8c5d8
to
3547471
Compare
@derekdreery where's this at? Should I give it a close read-through with the intention of merging? |
@cmyr There are some things that need tidying up, for example moving the code out into separate files and making sure everything matches the rest of the book. I would say that rather than a close review, you should first read it as a piece of text and see if it flows well & if it tells a coherent story. If you like it & think it is an improvement on what was there before, then we can worry about the details & getting it merged. |
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 like it, particularly the way that it introduces the lenses fully first, before explaining how they're integrated with druid.
## Fundamentals: Definition and Implementation | ||
|
||
Like Rust itself, lenses are one of those things that require effort up front to learn, but are very fun and effective to use once you understand them. This section represents the effort part of the equation. I promise if you stick with it you will reap the rewards. | ||
|
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 feels to me like it's repeating the previous paragraph.
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.
agree, I'm reminded of the old rub, "I would've written less but I didn't have time". I find in this sort of thing it's easy to write too much, and one of the challenges is going back and cutting stuff out.
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.
Some little initial comments, but maybe it's more interesting to focus on the higher-level stuff. Firstly: I realized halfway through this that I'm really not the best audience, since I have a pretty firm understanding of what's going on, and it's hard for me to tell what bits and pieces are helpful and which bits are too much.
I think we could put off showing the trait itself even further; I think the real challenge is coming up with a simple way of expressing the overall pattern. The concept I personally find most helpful is the idea of a "two-way map"; a way to both get a value out of a type, and also get a value back in.
One problem with Lens
is that the trait is so gross-looking. It's really almost an implementation detail that these things are called with
instead of get
; we only need with_mut
to take a function so that we have an opportunity to inspect the data afterwards and update our base state if we've done something fancy. If all lenses were field access, I think we could just have the methods be get
and get_mut
? (I haven't thought this through completely and it's possible that the closures also satisfy the borrow checker, but I don't think that's the case)..
So I think from a high-level the best way to explain this might just be as fancy getters and setters? I think a major part of why folks get confused is just the new name and the weird looking trait definition; the underlying concept just isn't that zany.
Anyway those are my high-level thoughts. I think that overall these docs are good, and I'd like to encourage you to fix them up and we'll get them merged.
@@ -1,149 +1,251 @@ | |||
# Lenses and the `Lens` trait | |||
|
|||
Let's say we're building a todo list application, and we are designing the widget | |||
that will represent a single todo item. Our data model looks like this: | |||
One of the key abstractions in `druid` along with `Data` is the `Lens` trait. This page explains what they are, and then how to use them. `Lens`es may seem complicated at first, but they are also very powerful, allowing you to write code that is reusable, concise, and understandable (once you understand `Lens`es themselves). |
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.
We've decided on Druid as the standard spelling for the project, only doing druid
if we're referring to the crate (as in, talking about druid
vs druid-shell
.
I'd also be fine with 'Lenses' once we're talking about the concept, and not the trait. 🤓
## Fundamentals: Definition and Implementation | ||
|
||
Like Rust itself, lenses are one of those things that require effort up front to learn, but are very fun and effective to use once you understand them. This section represents the effort part of the equation. I promise if you stick with it you will reap the rewards. | ||
|
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.
agree, I'm reminded of the old rub, "I would've written less but I didn't have time". I find in this sort of thing it's easy to write too much, and one of the challenges is going back and cutting stuff out.
|
||
## Conceptual | ||
The first thing to notice is the generics on the `Lens` itself. There are 3 types involve in the lens: the lens itself, `T` and `U`. The two type parameters represent the mis-match that lenses solve: we have a function that operates on `U`, and an object of type `T`, so we need to transform `T` into `U` somehow. |
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.
In technical writing I follow the principle of spelling out numerals up to at least ten.
|
||
`Lens` is a trait for types that perform this "focusing in" (aka *lensing*). | ||
A simplified version of the `Lens` trait might look like this: | ||
Time for an example. Let's implement & use `Lens` manually so we can see what's going on. |
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'd cut this down to something like, "Let's look at an example."
``` | ||
|
||
That is, this type takes an instance of `In`, and returns an instance of `Out`. | ||
This is a very simple case. All we need to do is project the function onto the field. Notice that this isn't the only vaid lens from `Container` to `String` we could have made - we could also project from `Container` to `another`. We made the choice how to transform `Container` into `String` when we implemented `Lens`. |
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 is a very simple case. All we need to do is project the function onto the field. Notice that this isn't the only vaid lens from `Container` to `String` we could have made - we could also project from `Container` to `another`. We made the choice how to transform `Container` into `String` when we implemented `Lens`. | |
This is a very simple case. All we need to do is project the function onto the field. Notice that this isn't the only valid lens from `Container` to `String` we could have made - we could also project from `Container` to `another`. We made the choice how to transform `Container` into `String` when we implemented `Lens`. |
I think I would just use a single field with the first example? I like the idea of keeping it maximally simple.
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 quite understand what "project the function onto the field" means here. I guess you're using project as a synonym for map
? Like map a field to another field? I only just thought of this, but Lensing is like mapping from one thing to another right?
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 agree that maybe 'project' is a bit too mathy? @arthmis you're correct, you can think of a lens as a two-way map; you can get one value from another, and then mutations in that value are also reflected in the original value.
struct Container { | ||
inner: String, | ||
another: String, | ||
} |
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'm of the mind of using more concrete example. The inner and outer fields are sort of vague to me, even though they are defined right there. When I read another
later on, I thought you were talking about something else. I was thinking you can use something like Contact
as the container and name
and email
as the fields. They would still be strings.
struct InnerLens; | ||
|
||
// Our lens will apply functions that operate on a `String` to a `Container`. | ||
impl Lens<Container, String> for InnerLens { | ||
fn with<F: FnOnce(&String)>(&self, data: &Container, f: F) { | ||
f(&data.inner); | ||
} | ||
|
||
fn with_mut<F: FnOnce(&mut String)>(&self, data: &mut Container, f: F) { | ||
f(&mut data.inner); | ||
} | ||
} |
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.
So, now InnerLens
can be renamed to NameLens
. Part of the issue I had reading this initially, was that I thought InnerLens meant lens that was internal rather than a lens that lenses into inner
. I just kept thinking inner was something else. This is my fault, but think it would be better to have more memorable field names for the example.
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.
Also maybe there should be an explanation of the paramter f: F
. What is that function doing under the hood?
This is unnecessary in the case of non-mutable access, but it is important for | ||
mutable access, because in many circumstances (such as when using an `Rc` or | ||
`Arc`) accessing a field mutably is expensive even if you don't do any mutation. | ||
We can actually do even better than this. Suppose that we are working on a vector of data rather than a string. We can import the `im` crate to get collections that use *structural sharing*, meaning that even when the vector is mutated, we only *Clone* what we need to. Because `im` is so useful, it is included in `druid` (behind the `im` feature). |
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.
Although the introduction of im
is important, I don't think it should be introduced in these docs. Maybe another example using the List widget would be nice. If anything I think there should be a sort of druid recipe book that demonstrates different types of ideas. i.e, different Lens
implementations, how to use them with lists and so on.
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.
Yeah I had thought about having a cookbook-style list of examples at the end. I think examples aid learning concepts, as well as being useful when you have a particular problem to solve and use a particular similar example.
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 Lens page should become a Lens chapter? After you've introduced the Lens
, you can continue with a plethora of different examples and demonstrate how they transform/map data. Even better if the examples are runnable.
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.
thanks for your comments @arthmis, it's super valuable to get feedback from someone who is less familiar with the subject matter!
|
||
### `LensExt` and combinators | ||
There are also 3 special structs in `druid::lens`: `Constant`, `Identity` and `Unit`. `Constant` is a lens that always returns the same value, and always discards any changes, while `Identity` is a lens that does nothing. You might say "what is the point of a lens that does nothing", which would be a fair question. Well, there are some places where a lens is required, and having an identity allows the user to say act as if there was no lens. It's also used to begin a composition chain using the combinators like `then`. `Unit` is a special case of `Constant` where the constant in question is `()`. |
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'm currently trying to understand how the lens works, and that's typically the sort of thing I'm looking to clear up: When to use Identity ? some idiomatic examples of combination with "LensExt" would be welcome. There is a case in "examples/list.rs" but it is not obvious to me as a novice.
@arthmis the cookbook model could work well: when starting out just search for the similar example to move you on, but as you do this more and more you start to see the patterns that facilitate understanding how |
I'm going to do another draft, but I think even then letting it stew some more is useful. Or if it gets merged, directing people somewhere to comment on it for future improvements. |
@derekdreery I agree that maybe lens by themselves don't need cookbook examples. I will try and submit some basic examples of lens inplementations and lens usage to you to see if you would find any useful. |
@arthmis awesome thanks! :) |
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 this is perfect, but the druid book is a work in progress anyway. I'm in favor of merging this for now, and improving later if need be.
This is a PR that rewrites the
Lens
page of the druid book. It aims to help avoid some of the misunderstandings I've seen on zulip. I've currently written it "green" without looking at the original page, and I intend to take another pass incorporating the best bits of both, but I think reviewing it in its current state would be a valuable endeavor. My main questions areLens
effectively, and in sufficient detail?Interested to hear what you think! Note that there are some technical things that need to happen if the PR is accepted:
lens
macro works, with an example of how code is expanded