-
Notifications
You must be signed in to change notification settings - Fork 19
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
Decomposition and normalisation #10
base: master
Are you sure you want to change the base?
Conversation
U know we have talked about it, I just did not know, if it fits there an where? Should I move it? |
Not sure if you are aware. But So if you define About |
@pevnak lets keep normalization here and move |
propname(::PropertyLens{name}) | ||
|
||
returns the name of the propery lens | ||
```julia |
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.
```julia | |
```jldoctest``` | |
Similar below. |
|
||
|
||
""" | ||
normalise(optic) |
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.
Lets spell it the American way normalize
to be consistent with julia base.
@@ -275,3 +275,63 @@ function show_composition_order(io::IO, lens::ComposedOptic) | |||
print(io, ")") | |||
end | |||
|
|||
|
|||
""" | |||
decompose(optic) |
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.
Should be moved to CompositionsBase.jl
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 looks good overall, just some minor remarks. Also, we should add a test, that normalize yields the compiler preferred composition order (I think this might be violated with your current implementation):
https://github.com/JuliaObjects/Accessors.jl/blob/master/test/perf.jl
@pevnak no idea if this is still relevant for you, but I added decompose here JuliaFunctional/CompositionsBase.jl#9 |
Is any part of this PR relevant now that |
Hi,
these are implementations of those two functions we have discussed in the Setfield thread.
They works as
and
I have add docs and tests.