Normalise optics when constructing VarName; remove extra constructors#123
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #123 +/- ##
==========================================
+ Coverage 82.43% 83.56% +1.12%
==========================================
Files 2 2
Lines 296 292 -4
==========================================
Hits 244 244
+ Misses 52 48 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request Test Coverage Report for Build 16098270412Details
💛 - Coveralls |
b70c160 to
1ffc9ad
Compare
498fc38 to
de2eccb
Compare
|
AbstractPPL.jl documentation for PR #123 is available at: |
de2eccb to
390ccd6
Compare
390ccd6 to
7bf3162
Compare
sunxd3
left a comment
There was a problem hiding this comment.
lgtm
maybe add a comment somewhere saying that ComposedOptics is just a alias of ComposedFunctions? (I opted for ComposedOptics when migrating to Accessors because I thought it's simpler to understand in context of the migration. But I support use ComposedFunction now.)
That makes sense. I guess it's really Julia that had me confused 😄 , since Accessors writes julia> using Accessors
julia> @o _.a.b
(@o _.a.b)
julia> dump(ans)
(@o _.a.b) (function of type ComposedFunction{PropertyLens{:b}, PropertyLens{:a}})
outer: PropertyLens{:b} (@o _.b)
inner: PropertyLens{:a} (@o _.a)I'll add a note! |
Closes #122.
I also added a changelog, because I've actually found it super helpful for DynamicPPL/Turing. I'm going to copy paste from it to describe this PR:
VarName(vn, optic)(this wasn't deprecated, but is IMO quite dangerous as it silently discards the existing optic invn), andVarName(vn, ::Tuple)(which was deprecated).Also changed all usage of
ComposedOptictoComposedFunction(it's the same thing: https://github.com/JuliaObjects/Accessors.jl/blob/18e9ddbc727ed4e48a3c19ffa00a02f7f407cdb5/src/optics.jl#L129).