Micro-optimise append_optic; fix serialisation for JSON@1.5.0 (#154)

* Micro-optimise `append_optic`

* Add tests

* tweak tests

* bump workflow versions

* Change OrderedDict -> NamedTuple

JuliaIO/JSON.jl#447

* Add changelog note

Author: penelopeysm

.github/workflows/CI.yml

@@ -31,13 +31,13 @@ jobs:
             os: macos-latest
 
     steps:
-      - uses: actions/checkout@v5
+      - uses: actions/checkout@v6
 
       - uses: julia-actions/setup-julia@v2
         with:
           version: ${{ matrix.runner.version }}
 
-      - uses: julia-actions/cache@v2
+      - uses: julia-actions/cache@v3
 
       - uses: julia-actions/julia-buildpkg@v1
 

HISTORY.md

@@ -1,3 +1,9 @@
+## 0.14.2
+
+Fix string serialisation of VarNames such that the order of keyword arguments is preserved (this was previously guaranteed, but JSON.jl v1.5.0 introduced a change that caused the keyword arguments to always be sorted.)
+
+Minor performance optimisation for `AbstractPPL.append_optic`.
+
 ## 0.14.1
 
 Export the `concretize_top_level` function, which concretizes only the indices contained in an `AbstractPPL.Index`, and does not recurse into child optics.

Project.toml

@@ -3,7 +3,7 @@ uuid = "7a57a42e-76ec-4ea3-a279-07e840d6d9cf"
 keywords = ["probablistic programming"]
 license = "MIT"
 desc = "Common interfaces for probabilistic programming"
-version = "0.14.1"
+version = "0.14.2"
 
 [deps]
 AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001"

src/varname/serialize.jl

@@ -124,7 +124,7 @@ function optic_to_dict(i::Index)
         # For some reason if you don't do the isempty check, it gets serialised as `{}`
         # rather than `[]`
         "ix" => isempty(i.ix) ? [] : collect(map(index_to_dict, i.ix)),
-        "kw" => OrderedDict(String(x) => index_to_dict(y) for (x, y) in pairs(i.kw)),
+        "kw" => NamedTuple{keys(i.kw)}(map(index_to_dict, values(i.kw))),
         "child" => optic_to_dict(i.child),
     )
 end

src/varname/varname.jl

@@ -400,3 +400,4 @@ x.a.b[1]
 function append_optic(vn::VarName{sym}, optic::AbstractOptic) where {sym}
     return VarName{sym}(cat(getoptic(vn), optic))
 end
+append_optic(vn::VarName, ::Iden) = vn

test/varname/serialize.jl

@@ -41,19 +41,11 @@ using Test
             @varname(x[j=2, i=1]),
             @varname(x[i=1, j=2]),
             @varname(x[].a[j=2].b[3, 4, 5, [6]]),
+            @varname(x[[1, 2, 5, 6]]),
         ]
         for vn in vns
             @test string_to_varname(varname_to_string(vn)) == vn
         end
-
-        # For this VarName, the {de,}serialisation works correctly but we must
-        # test in a different way because equality comparison of structs with
-        # vector fields (such as Accessors.IndexLens) compares the memory
-        # addresses rather than the contents (thus vn_vec == vn_vec2 returns
-        # false).
-        vn_vec = @varname(x[[1, 2, 5, 6]])
-        vn_vec2 = string_to_varname(varname_to_string(vn_vec))
-        @test hash(vn_vec) == hash(vn_vec2)
     end
 
     @testset "deserialisation fails for unconcretised dynamic indices" begin

test/varname/varname.jl

@@ -226,6 +226,8 @@ using JET: @test_call
         @test append_optic(@varname(x), @opticof(_[1])) == @varname(x[1])
         @test append_optic(@varname(x.a), @opticof(_[1])) == @varname(x.a[1])
         @test append_optic(@varname(x[1]), @opticof(_.a)) == @varname(x[1].a)
+        @test append_optic(@varname(x), @opticof(_)) == @varname(x)
+        @test append_optic(@varname(x.a), @opticof(_)) == @varname(x.a)
     end
 end