From 8a60ba95c9240a2ba87a88e817f985641cfd4161 Mon Sep 17 00:00:00 2001 From: Alexander Plavin Date: Sun, 3 Mar 2024 11:12:20 -0500 Subject: [PATCH 1/6] more CI versions --- .github/workflows/ci.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 36f4b09c..44933fa8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,13 +10,17 @@ jobs: fail-fast: false matrix: version: - - 1.6 # LTS - - 1 + - 1.6 + - 1.7 + - 1.8 + - 1.9 + - '1.10' + - '1.11-nightly' - 'nightly' os: - ubuntu-latest # - macOS-latest - - windows-latest + # - windows-latest arch: - x64 steps: From 52bd526adb5c669cab9984400aaa63b2a01f09cd Mon Sep 17 00:00:00 2001 From: Alexander Plavin Date: Sun, 3 Mar 2024 10:58:38 -0500 Subject: [PATCH 2/6] test: nightly bug --- test/test_core.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_core.jl b/test/test_core.jl index b292456d..adf659ef 100644 --- a/test/test_core.jl +++ b/test/test_core.jl @@ -601,7 +601,7 @@ end @testset "@accessor" begin s = MyStruct((a=123,)) - @test strip(string(@doc(my_x))) == "Documentation for my_x" + # @test strip(string(@doc(my_x))) == "Documentation for my_x" @test (@set my_x(s) = 456) === MyStruct(456) @test (@set +s = 456) === MyStruct((a=5-456,)) test_getset_laws(my_x, s, 456, "1") From 52e1e8fbcd06dbc1ebce123442e069d4aca37367 Mon Sep 17 00:00:00 2001 From: Alexander Plavin Date: Sun, 3 Mar 2024 11:28:53 -0500 Subject: [PATCH 3/6] remove OpticStyle --- examples/custom_optics.jl | 1 - examples/specter.jl | 5 +-- src/getsetall.jl | 4 +- src/optics.jl | 84 +++++++-------------------------------- 4 files changed, 18 insertions(+), 76 deletions(-) diff --git a/examples/custom_optics.jl b/examples/custom_optics.jl index d51a0c48..86bca7f4 100644 --- a/examples/custom_optics.jl +++ b/examples/custom_optics.jl @@ -56,7 +56,6 @@ end # using Accessors struct Keys end -Accessors.OpticStyle(::Type{Keys}) = ModifyBased() Accessors.modify(f, obj, ::Keys) = mapkeys(f, obj) # It can be used as follows: obj = Dict("A" =>1, "B" => 2, "C" => 3) diff --git a/examples/specter.jl b/examples/specter.jl index 9d058fdf..c1176347 100644 --- a/examples/specter.jl +++ b/examples/specter.jl @@ -8,8 +8,8 @@ using Test using Accessors -import Accessors: modify, OpticStyle -using Accessors: ModifyBased, SetBased, setindex +import Accessors: modify +using Accessors: setindex # ### Increment all even numbers # We have the following data and the goal is to increment all nested even numbers. @@ -22,7 +22,6 @@ end mapvals(f, nt::NamedTuple) = map(f, nt) struct Vals end -OpticStyle(::Type{Vals}) = ModifyBased() modify(f, obj, ::Vals) = mapvals(f, obj) # Now we can increment as follows: diff --git a/src/getsetall.jl b/src/getsetall.jl index b5e3929d..240dcdcb 100644 --- a/src/getsetall.jl +++ b/src/getsetall.jl @@ -61,7 +61,7 @@ getall(obj::AbstractString, ::Elements) = collect(obj) getall(obj, ::Elements) = error("Elements() not supported for $(typeof(obj))") getall(obj, ::Properties) = getproperties(obj) |> values getall(obj, o::If) = o.modify_condition(obj) ? (obj,) : () -getall(obj, o) = OpticStyle(o) == SetBased() ? (o(obj),) : error("`getall` not supported for $o") +getall(obj, o) = (o(obj),) function setall(obj, ::Properties, vs) names = propertynames(obj) @@ -84,7 +84,7 @@ function setall(obj, o::If, vs) obj end end -setall(obj, o, vs) = OpticStyle(o) == SetBased() ? set(obj, o, only(vs)) : error("`setall` not supported for $o") +setall(obj, o, vs) = set(obj, o, only(vs)) # implementations for composite optics diff --git a/src/optics.jl b/src/optics.jl index dee543ff..4f084e1f 100644 --- a/src/optics.jl +++ b/src/optics.jl @@ -130,49 +130,12 @@ const ComposedOptic{Outer,Inner} = ComposedFunction{Outer,Inner} outertype(::Type{ComposedOptic{Outer,Inner}}) where {Outer,Inner} = Outer innertype(::Type{ComposedOptic{Outer,Inner}}) where {Outer,Inner} = Inner -# TODO better name -# also better way to organize traits will -# probably only emerge over time -# -# TODO -# There is an inference regression as of Julia v1.7.0 -# if recursion is combined with trait based dispatch -# https://github.com/JuliaLang/julia/issues/43296 - -abstract type OpticStyle end -struct ModifyBased <: OpticStyle end -struct SetBased <: OpticStyle end -# Base.@pure OpticStyle(obj) = OpticStyle(typeof(obj)) -function OpticStyle(optic::T) where {T} - OpticStyle(T) -end -# defining lenses should be very lightweight -# e.g. only a single `set` implementation -# so we choose this as the default trait -OpticStyle(::Type{T}) where {T} = SetBased() - -function OpticStyle(::Type{ComposedOptic{O,I}}) where {O,I} - composed_optic_style(OpticStyle(O), OpticStyle(I)) -end -composed_optic_style(::SetBased, ::SetBased) = SetBased() -composed_optic_style(::ModifyBased, ::SetBased) = ModifyBased() -composed_optic_style(::SetBased, ::ModifyBased) = ModifyBased() -composed_optic_style(::ModifyBased, ::ModifyBased) = ModifyBased() - @inline function set(obj, optic::O, val) where {O} - _set(obj, optic, val, OpticStyle(O)) -end - -function _set(obj, optic, val, ::SetBased) inv_func = inverse(optic) if !(inv_func isa NoInverse) return inv_func(val) end - Optic = typeof(optic) - error(""" - This should be unreachable. You probably need to overload - `Accessors.set(obj, ::$Optic, val) - """) + modify(Returns(val), obj, optic) end if VERSION < v"1.7" @@ -184,39 +147,24 @@ else using Base: Returns end -@inline function _set(obj, optic, val, ::ModifyBased) - modify(Returns(val), obj, optic) -end - -@inline function _set(obj, optic::ComposedOptic, val, ::SetBased) - inner_obj = optic.inner(obj) - inner_val = set(inner_obj, optic.outer, val) - set(obj, optic.inner, inner_val) -end - @inline function modify(f, obj, optic::O) where {O} - _modify(f, obj, optic, OpticStyle(O)) -end - -function _modify(f, obj, optic, ::ModifyBased) - Optic = typeof(optic) - error(""" - This should be unreachable. You probably need to overload: - `Accessors.modify(f, obj, ::$Optic)` - """) + set(obj, optic, f(optic(obj))) end -function _modify(f, obj, optic::ComposedOptic, ::ModifyBased) - otr = optic.outer - inr = optic.inner - modify(obj, inr) do o1 - modify(f, o1, otr) +@inline function set(obj, optic::ComposedOptic, val) + optics = decompose(optic) + _modifyc(obj, Base.tail(optics)) do o1 + set(o1, first(optics), val) end end +@inline modify(f, obj, optic::ComposedOptic) = _modifyc(f, obj, decompose(optic)) -@inline function _modify(f, obj, optic, ::SetBased) - set(obj, optic, f(optic(obj))) -end +@inline _modifyc(f, obj, os::Tuple{}) = f(obj) +@inline _modifyc(f, obj, os::Tuple{Any}) = modify(f, obj, only(os)) +@inline _modifyc(f, obj, os::Tuple) = + modify(obj, last(os)) do o1 + _modifyc(f, o1, Base.front(os)) + end function delete(obj, optic::ComposedOptic) modify(obj, optic.inner) do inner_obj @@ -249,7 +197,6 @@ julia> modify(x -> 2x, obj, Elements()) ``` """ struct Elements end -OpticStyle(::Type{<:Elements}) = ModifyBased() modify(f, obj, ::Elements) = map(f, obj) # sets and dicts don't support map(), but still have the concept of elements: @@ -275,7 +222,6 @@ $EXPERIMENTAL struct If{C} modify_condition::C end -OpticStyle(::Type{<:If}) = ModifyBased() function modify(f, obj, w::If) if w.modify_condition(obj) @@ -338,7 +284,6 @@ julia> modify(x -> 2x, obj, Properties()) Based on [`mapproperties`](@ref). """ struct Properties end -OpticStyle(::Type{<:Properties}) = ModifyBased() modify(f, o, ::Properties) = mapproperties(f, o) """ @@ -365,9 +310,8 @@ struct Recursive{Descent,Optic} descent_condition::Descent optic::Optic end -OpticStyle(::Type{Recursive{D,O}}) where {D,O} = ModifyBased() # Is this a good idea? -function _modify(f, obj, r::Recursive, ::ModifyBased) +function modify(f, obj, r::Recursive) modify(obj, r.optic) do o if r.descent_condition(o) modify(f, o, r) From d509019dbe0b3b14bd334449d583df360dd36750 Mon Sep 17 00:00:00 2001 From: Alexander Plavin Date: Sun, 3 Mar 2024 11:25:45 -0500 Subject: [PATCH 4/6] tests: wrong exceptions --- test/test_core.jl | 4 ++-- test/test_extensions.jl | 4 ++-- test/test_functionlenses.jl | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/test_core.jl b/test/test_core.jl index adf659ef..abbe0f0e 100644 --- a/test/test_core.jl +++ b/test/test_core.jl @@ -55,11 +55,11 @@ end s = @set t.b.b.a.a = 5 @test t === T(1, T(2, T(T(4,4),3))) @test s === T(1, T(2, T(T(5, 4), 3))) - @test_throws ArgumentError @set t.b.b.a.a.a = 3 + @test_throws Exception @set t.b.b.a.a.a = 3 t = T(1,2) @test T(1, T(1,2)) === @set t.b = T(1,2) - @test_throws ArgumentError @set t.c = 3 + @test_throws Exception @set t.c = 3 t = T(T(2,2), 1) s = @set t.a.a = 3 diff --git a/test/test_extensions.jl b/test/test_extensions.jl index a351fa6f..2467a125 100644 --- a/test/test_extensions.jl +++ b/test/test_extensions.jl @@ -155,7 +155,7 @@ VERSION >= v"1.9-" && @testset "StructArrays" begin sa = @set sb.a = 1:3 @test sa.a === 1:3 @test sa.b === sb.b - @test_throws ArgumentError @set sb.c = 1:3 + @test_throws Exception @set sb.c = 1:3 sd = @delete sb.a @test sd::StructArray == StructArray(b=10:12) @test_throws "only eltypes with fields" @delete s.a @@ -172,7 +172,7 @@ VERSION >= v"1.9-" && @testset "StructArrays" begin @test @inferred(set(s, PropertyLens(:a), 10:11))::StructArray == StructArray([S(10, 2), S(11, 4)]) @test @inferred(set(s, PropertyLens(:a), [:a, :b]))::StructArray == StructArray([S(:a, 2), S(:b, 4)]) - @test_throws "need to overload" set(s, propertynames, (:x, :y)) + @test_throws Exception set(s, propertynames, (:x, :y)) s = StructArray(x=[1, 2], y=[:a, :b]) test_getset_laws(propertynames, s, (:u, :v), (1, 2)) test_getset_laws(propertynames, s, (1, 2), (:u, :v)) diff --git a/test/test_functionlenses.jl b/test/test_functionlenses.jl index c9c9947c..ac6566a8 100644 --- a/test/test_functionlenses.jl +++ b/test/test_functionlenses.jl @@ -284,7 +284,7 @@ end # @optic(parse(Int, _)) isa Base.Fix1{typeof(parse), Type{T}} where {T} # doesn't hold @test @inferred(modify(x -> -2x, "3", @optic parse(Int, _))) == "-6" - @test_throws ErrorException modify(log10, "100", @optic parse(Int, _)) + @test_throws Exception modify(log10, "100", @optic parse(Int, _)) @test modify(log10, "100", @optic parse(Float64, _)) == "2.0" test_getset_laws(@optic(parse(Int, _)), "3", -10, 123) test_getset_laws(@optic(parse(Float64, _)), "3.0", -10., 123.) From 66aa4531ff8e3d918ecb0b54b188368a4a3d0b37 Mon Sep 17 00:00:00 2001 From: Alexander Plavin Date: Sun, 3 Mar 2024 11:38:17 -0500 Subject: [PATCH 5/6] tests: improvements --- test/perf.jl | 6 ++---- test/test_core.jl | 16 +++------------- test/test_functionlenses.jl | 8 ++++++++ 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/test/perf.jl b/test/perf.jl index 158d213d..4a2e2e97 100644 --- a/test/perf.jl +++ b/test/perf.jl @@ -177,16 +177,14 @@ end println("Right associative composition: $b_right") @test b_default.allocs == 0 - if VERSION >= v"1.10-" - @test_broken b_right.allocs == 0 - elseif VERSION >= v"1.7" + if VERSION >= v"1.7" @test b_right.allocs == 0 else @test_broken right.allocs == 0 - @test b_right.time > 2b_default.time end @test b_left.allocs == 0 @test b_left.time ≈ b_default.time rtol=0.8 + @test b_right.time ≈ b_default.time rtol=0.8 end end diff --git a/test/test_core.jl b/test/test_core.jl index abbe0f0e..57209b70 100644 --- a/test/test_core.jl +++ b/test/test_core.jl @@ -220,19 +220,9 @@ end ((@optic _.b.a.b[end]), 4.0), ((@optic _.b.a.b[end÷2+1]), 4.0), ] - if VERSION < v"1.7" || VERSION >= v"1.10-" - @inferred lens(obj) - @inferred set(obj, lens, val) - @inferred modify(identity, obj, lens) - else - @inferred lens(obj) - @inferred set(obj, lens, val) - @test_broken begin - # https://github.com/JuliaLang/julia/issues/43296 - @inferred modify(identity, obj, lens) - true - end - end + @inferred lens(obj) + @inferred set(obj, lens, val) + @inferred modify(identity, obj, lens) end end diff --git a/test/test_functionlenses.jl b/test/test_functionlenses.jl index ac6566a8..1052a322 100644 --- a/test/test_functionlenses.jl +++ b/test/test_functionlenses.jl @@ -391,4 +391,12 @@ end test_getset_laws(o2, x, 2, -3) end +@testset "non-callable" begin + struct MyF end + Accessors.set(x, ::MyF, y) = y + 1 + Accessors.modify(f, x, ::MyF) = f(x) + @test set(1, (@o _ + 2 |> MyF()), 3) == 2 + @test modify(x -> 10x, 1, (@o _ + 2 |> MyF())) == 28 +end + end # module From a0afef7806ad774f95b78472b6a1ee5e4779d1aa Mon Sep 17 00:00:00 2001 From: Alexander Plavin Date: Sun, 3 Mar 2024 11:29:13 -0500 Subject: [PATCH 6/6] use @eval --- src/optics.jl | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/optics.jl b/src/optics.jl index 4f084e1f..51456be2 100644 --- a/src/optics.jl +++ b/src/optics.jl @@ -160,11 +160,12 @@ end @inline modify(f, obj, optic::ComposedOptic) = _modifyc(f, obj, decompose(optic)) @inline _modifyc(f, obj, os::Tuple{}) = f(obj) -@inline _modifyc(f, obj, os::Tuple{Any}) = modify(f, obj, only(os)) -@inline _modifyc(f, obj, os::Tuple) = - modify(obj, last(os)) do o1 - _modifyc(f, o1, Base.front(os)) - end +for N in [1:10; :(<: Any)] + @eval @inline _modifyc(f, obj, os::NTuple{$N,Any}) = + modify(obj, last(os)) do o1 + _modifyc(f, o1, Base.front(os)) + end +end function delete(obj, optic::ComposedOptic) modify(obj, optic.inner) do inner_obj