From 9156b3c9f5e42c274446541583eb4cc352206775 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 12 Sep 2024 01:38:35 +0100
Subject: [PATCH 01/21] Fix typo in comment

---
 src/varname.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/varname.jl b/src/varname.jl
index 1ffa542..b4960ef 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -302,7 +302,7 @@ subsumes(t::ComposedOptic, u::ComposedOptic) =
 # If `t` is still a composed lens, then there is no way it can subsume `u` since `u` is a
 # leaf of the "lens-tree".
 subsumes(t::ComposedOptic, u::PropertyLens) = false
-# Here we need to check if `u.outer` (i.e. the next lens to be applied from `u`) is
+# Here we need to check if `u.inner` (i.e. the next lens to be applied from `u`) is
 # subsumed by `t`, since this would mean that the rest of the composition is also subsumed
 # by `t`.
 subsumes(t::PropertyLens, u::ComposedOptic) = subsumes(t, u.inner)

From a984dc501804613e2c5f726c6daaaf4cf040c1ec Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 12 Sep 2024 17:56:41 +0100
Subject: [PATCH 02/21] Conversion of VarName to/from string

---
 src/AbstractPPL.jl |  1 +
 src/varname.jl     |  2 ++
 test/varname.jl    | 30 ++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/src/AbstractPPL.jl b/src/AbstractPPL.jl
index 775576a..012364f 100644
--- a/src/AbstractPPL.jl
+++ b/src/AbstractPPL.jl
@@ -8,6 +8,7 @@ export VarName,
     subsumes,
     subsumedby,
     varname,
+    varname_from_str,
     vsym,
     @varname,
     @vsym
diff --git a/src/varname.jl b/src/varname.jl
index b4960ef..dc12c76 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -663,6 +663,8 @@ function drop_escape(expr::Expr)
     return Expr(expr.head, map(x -> drop_escape(x), expr.args)...)
 end
 
+varname_from_str(str::AbstractString) = eval(drop_escape(varname(Meta.parse(str))))
+
 function _parse_obj_optic(ex)
     obj, optics = _parse_obj_optics(ex)
     optic = Expr(:call, Accessors.opticcompose, optics...)
diff --git a/test/varname.jl b/test/varname.jl
index d0e3de7..4ca7912 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -137,4 +137,34 @@ end
         @inferred get(c, @varname(b.a[1]))
         @inferred Accessors.set(c, @varname(b.a[1]), 10)
     end
+
+    @testset "roundtrip conversion to/from string" begin
+        # Static optics
+        vns = [
+            @varname(x),
+            @varname(x.a),
+            @varname(x[1]),
+            @varname(x[1:10]),
+            @varname(x[1, 2]),
+            @varname(x[1, 2:5]),
+            @varname(x[:]),
+            @varname(x.a[1]),
+            @varname(x.a[1:10]),
+            @varname(x[1].a),
+        ]
+        for vn in vns
+            @test varname_from_str(repr(vn)) == vn
+        end
+
+        # Post-concretisation
+        x = ones(10)
+        vn = @varname(x[begin:end])
+        @test varname_from_str(repr(vn)) == vn
+
+        # When forcing concretisation
+        vn = @varname(x[:], true)
+        @test_broken varname_from_str(repr(vn)) == vn
+        dump(varname_from_str(repr(vn)))
+        dump(vn)
+    end
 end

From 4cbc104df78966b3de5bf971fc42150e7e5fe2d2 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 12 Sep 2024 22:23:44 +0100
Subject: [PATCH 03/21] Add one more test

---
 test/varname.jl | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/test/varname.jl b/test/varname.jl
index 4ca7912..5841f02 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -156,12 +156,19 @@ end
             @test varname_from_str(repr(vn)) == vn
         end
 
+        # Variables
+        i = 10
+        vn = @varname(x[i])
+        @test varname_from_str(repr(vn)) == vn
+
         # Post-concretisation
         x = ones(10)
         vn = @varname(x[begin:end])
         @test varname_from_str(repr(vn)) == vn
 
         # When forcing concretisation
+        # This is broken. Should we have a repr_concretised() method or
+        # something like that?
         vn = @varname(x[:], true)
         @test_broken varname_from_str(repr(vn)) == vn
         dump(varname_from_str(repr(vn)))

From b6625605504139f6f69e9a013e5e83b163900527 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Tue, 17 Sep 2024 11:58:03 +0100
Subject: [PATCH 04/21] More thorough serialisation

---
 src/AbstractPPL.jl |  3 ++-
 src/varname.jl     | 62 ++++++++++++++++++++++++++++++++++++++++++++--
 test/varname.jl    | 31 ++++++++---------------
 3 files changed, 73 insertions(+), 23 deletions(-)

diff --git a/src/AbstractPPL.jl b/src/AbstractPPL.jl
index 012364f..ca53642 100644
--- a/src/AbstractPPL.jl
+++ b/src/AbstractPPL.jl
@@ -8,7 +8,8 @@ export VarName,
     subsumes,
     subsumedby,
     varname,
-    varname_from_str,
+    vn_to_string,
+    vn_from_string,
     vsym,
     @varname,
     @vsym
diff --git a/src/varname.jl b/src/varname.jl
index dc12c76..2059fc3 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -450,6 +450,7 @@ struct ConcretizedSlice{T,R} <: AbstractVector{T}
 end
 
 ConcretizedSlice(s::Base.Slice{R}) where {R} = ConcretizedSlice{eltype(s.indices),R}(s.indices)
+ConcretizedSlice(s::Base.OneTo{R}) where {R} = ConcretizedSlice(Base.Slice(s))
 Base.show(io::IO, s::ConcretizedSlice) = print(io, ":")
 Base.show(io::IO, ::MIME"text/plain", s::ConcretizedSlice) =
     print(io, "ConcretizedSlice(", s.range, ")")
@@ -663,8 +664,6 @@ function drop_escape(expr::Expr)
     return Expr(expr.head, map(x -> drop_escape(x), expr.args)...)
 end
 
-varname_from_str(str::AbstractString) = eval(drop_escape(varname(Meta.parse(str))))
-
 function _parse_obj_optic(ex)
     obj, optics = _parse_obj_optics(ex)
     optic = Expr(:call, Accessors.opticcompose, optics...)
@@ -749,3 +748,62 @@ function vsym(expr::Expr)
         error("Malformed variable name `$(expr)`!")
     end
 end
+
+"""
+    index_to_str(i)
+
+Generates a string representation of the index `i`, or a tuple thereof.
+"""
+index_to_str(i::Integer) = string(i)
+index_to_str(r::UnitRange) = "$(first(r)):$(last(r))"
+index_to_str(::Colon) = ":"
+index_to_str(s::ConcretizedSlice{T,R}) where {T,R} = "ConcretizedSlice(" * repr(s.range) * ")"
+index_to_str(t::Tuple) = "(" * join(map(index_to_str, t), ", ") * ",)"
+
+"""
+    optic_to_nt(optic)
+
+Convert an optic to a named tuple representation.
+"""
+optic_to_nt(::typeof(identity)) = (type = "identity",)
+optic_to_nt(::PropertyLens{sym}) where {sym} = (type = "property", field = String(sym))
+optic_to_nt(i::IndexLens) = (type = "index", indices = index_to_str(i.indices))
+optic_to_nt(c::ComposedOptic) = (type = "composed", outer = optic_to_nt(c.outer), inner = optic_to_nt(c.inner))
+
+
+"""
+    nt_to_optic(nt)
+
+Convert a named tuple representation back to an optic.
+"""
+function nt_to_optic(nt)
+    if nt.type == "identity"
+        return identity
+    elseif nt.type == "index"
+        return IndexLens(eval(Meta.parse(nt.indices)))
+    elseif nt.type == "property"
+        return PropertyLens{Symbol(nt.field)}()
+    elseif nt.type == "composed"
+        return nt_to_optic(nt.outer) ∘ nt_to_optic(nt.inner)
+    end
+end
+
+"""
+    vn_to_string(vn::VarName)
+
+Convert a `VarName` as a string (via an intermediate named tuple).
+"""
+vn_to_string(vn::VarName) = repr((sym = String(getsym(vn)), optic = optic_to_nt(getoptic(vn))))
+
+"""
+    vn_from_string(str)
+
+Convert a string representation of a `VarName` back to a `VarName`.
+
+NOTE: This function should only be used with trusted input, as it uses `eval`
+and `Meta.parse` to parse the string.
+"""
+function vn_from_string(str)
+    new_fields = eval(Meta.parse(str))
+    return VarName{Symbol(new_fields.sym)}(nt_to_optic(new_fields.optic))
+end
diff --git a/test/varname.jl b/test/varname.jl
index 5841f02..f1d7742 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -139,11 +139,15 @@ end
     end
 
     @testset "roundtrip conversion to/from string" begin
-        # Static optics
+        y = ones(10)
         vns = [
             @varname(x),
+            @varname(ä),
             @varname(x.a),
+            @varname(x.a.b),
+            @varname(var"x.a"),
             @varname(x[1]),
+            @varname(var"x[1]"),
             @varname(x[1:10]),
             @varname(x[1, 2]),
             @varname(x[1, 2:5]),
@@ -151,27 +155,14 @@ end
             @varname(x.a[1]),
             @varname(x.a[1:10]),
             @varname(x[1].a),
+            @varname(y[:]),
+            @varname(y[begin:end]),
+            @varname(y[end]),
+            @varname(y[:], false),
+            @varname(y[:], true),
         ]
         for vn in vns
-            @test varname_from_str(repr(vn)) == vn
+            @test vn_from_string(vn_to_string(vn)) == vn
         end
-
-        # Variables
-        i = 10
-        vn = @varname(x[i])
-        @test varname_from_str(repr(vn)) == vn
-
-        # Post-concretisation
-        x = ones(10)
-        vn = @varname(x[begin:end])
-        @test varname_from_str(repr(vn)) == vn
-
-        # When forcing concretisation
-        # This is broken. Should we have a repr_concretised() method or
-        # something like that?
-        vn = @varname(x[:], true)
-        @test_broken varname_from_str(repr(vn)) == vn
-        dump(varname_from_str(repr(vn)))
-        dump(vn)
     end
 end

From 1821419dc21ff3ded05f62529cb44ea0d8eb8807 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Tue, 17 Sep 2024 14:24:35 +0100
Subject: [PATCH 05/21] Add doctests

---
 src/varname.jl | 52 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/src/varname.jl b/src/varname.jl
index 2059fc3..bfa7ede 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -753,6 +753,22 @@ end
     index_to_str(i)
 
 Generates a string representation of the index `i`, or a tuple thereof.
+
+## Examples
+
+```jldoctest
+julia> index_to_str(2)
+"2"
+
+julia> index_to_str((1, 2:5))
+"(1, 2:5,)"
+
+julia> index_to_str(:)
+":"
+
+julia> index_to_str(ConcretizedSlice(Base.Slice(Base.OneTo(10))))
+"ConcretizedSlice(Base.OneTo(10))"
+```
 """
 index_to_str(i::Integer) = string(i)
 index_to_str(r::UnitRange) = "$(first(r)):$(last(r))"
@@ -764,6 +780,21 @@ index_to_str(t::Tuple) = "(" * join(map(index_to_str, t), ", ") * ",)"
     optic_to_nt(optic)
 
 Convert an optic to a named tuple representation.
+
+## Examples
+```jldoctest; setup=:(using Accessors)
+julia> optic_to_nt(identity)
+(type = "identity",)
+
+julia> optic_to_nt(@optic _.a)
+(type = "property", field = "a")
+
+julia> optic_to_nt(@optic _.a.b)
+(type = "composed", outer = (type = "property", field = "b"), inner = (type = "property", field = "a"))
+
+julia> optic_to_nt(@optic _[1])  # uses index_to_str()
+(type = "index", indices = "(1,)")
+```
 """
 optic_to_nt(::typeof(identity)) = (type = "identity",)
 optic_to_nt(::PropertyLens{sym}) where {sym} = (type = "property", field = String(sym))
@@ -791,14 +822,31 @@ end
 """
     vn_to_string(vn::VarName)
 
-Convert a `VarName` as a string (via an intermediate named tuple).
+Convert a `VarName` as a string, via an intermediate named tuple. This differs
+from `string(vn)` in that concretised slices are faithfully represented (rather
+than being pretty-printed as colons).
+
+```jldoctest
+julia> vn_to_string(@varname(x))
+"(sym = \"x\", optic = (type = \"identity\",))"
+
+julia> vn_to_string(@varname(x.a))
+"(sym = \"x\", optic = (type = \"property\", field = \"a\"))"
+
+julia> y = ones(2); vn_to_string(@varname(y[:]))
+"(sym = \"y\", optic = (type = \"index\", indices = \"(:,)\"))"
+
+julia> y = ones(2); vn_to_string(@varname(y[:], true))
+"(sym = \"y\", optic = (type = \"index\", indices = \"(ConcretizedSlice(Base.OneTo(2)),)\"))"
+```
 """
 vn_to_string(vn::VarName) = repr((sym = String(getsym(vn)), optic = optic_to_nt(getoptic(vn))))
 
 """
     vn_from_string(str)
 
-Convert a string representation of a `VarName` back to a `VarName`.
+Convert a string representation of a `VarName` back to a `VarName`. The string
+should have been generated by `vn_to_string`.
 
 NOTE: This function should only be used with trusted input, as it uses `eval`
 and `Meta.parse` to parse the string.

From 80af762fcb2d8528ebd3a0b3f8ba322d5bdb16bd Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Wed, 18 Sep 2024 14:05:32 +0100
Subject: [PATCH 06/21] Add API docs

---
 docs/src/api.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/src/api.md b/docs/src/api.md
index a8d4321..3507419 100644
--- a/docs/src/api.md
+++ b/docs/src/api.md
@@ -12,6 +12,8 @@ subsumedby
 vsym
 @varname
 @vsym
+vn_to_string
+vn_from_string
 ```
 
 ## Abstract model functions

From 2146e2282c9940b959cb77d9834aacf1e7f6b2d4 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Wed, 18 Sep 2024 17:37:58 +0100
Subject: [PATCH 07/21] Add warning to docstring

Co-authored-by: Tor Erlend Fjelde <tor.erlend95@gmail.com>
---
 src/varname.jl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/varname.jl b/src/varname.jl
index d43b790..717d2b6 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -853,7 +853,8 @@ vn_to_string(vn::VarName) = repr((sym = String(getsym(vn)), optic = optic_to_nt(
 Convert a string representation of a `VarName` back to a `VarName`. The string
 should have been generated by `vn_to_string`.
 
-NOTE: This function should only be used with trusted input, as it uses `eval`
+!!! warning
+    This function should only be used with trusted input, as it uses `eval`
 and `Meta.parse` to parse the string.
 """
 function vn_from_string(str)

From d7ef8d7835171c8b3cd4d5ccfe5ade751f5d4727 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 26 Sep 2024 15:59:32 +0100
Subject: [PATCH 08/21] Add alternate implementation with StructTypes

---
 Project.toml       |  2 ++
 src/AbstractPPL.jl |  2 ++
 src/varname.jl     | 74 ++++++++++++++++++++++++++++++++++++++--------
 test/varname.jl    |  2 +-
 4 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/Project.toml b/Project.toml
index b32ad51..58f1dc5 100644
--- a/Project.toml
+++ b/Project.toml
@@ -9,7 +9,9 @@ version = "0.8.4"
 AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001"
 Accessors = "7d9f7c33-5ae7-4f3b-8dc6-eff91059b697"
 DensityInterface = "b429d917-457f-4dbc-8f4c-0cc954292b1d"
+JSON3 = "0f8b85d8-7281-11e9-16c2-39a750bddbf1"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
+StructTypes = "856f2bd8-1eba-4b0a-8007-ebc267875bd4"
 
 [compat]
 AbstractMCMC = "2, 3, 4, 5"
diff --git a/src/AbstractPPL.jl b/src/AbstractPPL.jl
index ca53642..9a4a768 100644
--- a/src/AbstractPPL.jl
+++ b/src/AbstractPPL.jl
@@ -10,6 +10,8 @@ export VarName,
     varname,
     vn_to_string,
     vn_from_string,
+    vn_to_string2,
+    vn_from_string2,
     vsym,
     @varname,
     @vsym
diff --git a/src/varname.jl b/src/varname.jl
index 717d2b6..15782b6 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -1,5 +1,7 @@
 using Accessors
 using Accessors: ComposedOptic, PropertyLens, IndexLens, DynamicIndexLens
+using StructTypes: StructTypes
+using JSON3: JSON3
 
 const ALLOWED_OPTICS = Union{typeof(identity),PropertyLens,IndexLens,ComposedOptic}
 
@@ -762,16 +764,16 @@ Generates a string representation of the index `i`, or a tuple thereof.
 ## Examples
 
 ```jldoctest
-julia> index_to_str(2)
+julia> AbstractPPL.index_to_str(2)
 "2"
 
-julia> index_to_str((1, 2:5))
+julia> AbstractPPL.index_to_str((1, 2:5))
 "(1, 2:5,)"
 
-julia> index_to_str(:)
+julia> AbstractPPL.index_to_str(:)
 ":"
 
-julia> index_to_str(ConcretizedSlice(Base.Slice(Base.OneTo(10))))
+julia> AbstractPPL.index_to_str(AbstractPPL.ConcretizedSlice(Base.Slice(Base.OneTo(10))))
 "ConcretizedSlice(Base.OneTo(10))"
 ```
 """
@@ -788,16 +790,16 @@ Convert an optic to a named tuple representation.
 
 ## Examples
 ```jldoctest; setup=:(using Accessors)
-julia> optic_to_nt(identity)
+julia> AbstractPPL.optic_to_nt(identity)
 (type = "identity",)
 
-julia> optic_to_nt(@optic _.a)
+julia> AbstractPPL.optic_to_nt(@optic _.a)
 (type = "property", field = "a")
 
-julia> optic_to_nt(@optic _.a.b)
+julia> AbstractPPL.optic_to_nt(@optic _.a.b)
 (type = "composed", outer = (type = "property", field = "b"), inner = (type = "property", field = "a"))
 
-julia> optic_to_nt(@optic _[1])  # uses index_to_str()
+julia> AbstractPPL.optic_to_nt(@optic _[1])  # uses index_to_str()
 (type = "index", indices = "(1,)")
 ```
 """
@@ -833,16 +835,16 @@ than being pretty-printed as colons).
 
 ```jldoctest
 julia> vn_to_string(@varname(x))
-"(sym = \"x\", optic = (type = \"identity\",))"
+"(sym = \\"x\\", optic = (type = \\"identity\\",))"
 
 julia> vn_to_string(@varname(x.a))
-"(sym = \"x\", optic = (type = \"property\", field = \"a\"))"
+"(sym = \\"x\\", optic = (type = \\"property\\", field = \\"a\\"))"
 
 julia> y = ones(2); vn_to_string(@varname(y[:]))
-"(sym = \"y\", optic = (type = \"index\", indices = \"(:,)\"))"
+"(sym = \\"y\\", optic = (type = \\"index\\", indices = \\"(:,)\\"))"
 
 julia> y = ones(2); vn_to_string(@varname(y[:], true))
-"(sym = \"y\", optic = (type = \"index\", indices = \"(ConcretizedSlice(Base.OneTo(2)),)\"))"
+"(sym = \\"y\\", optic = (type = \\"index\\", indices = \\"(ConcretizedSlice(Base.OneTo(2)),)\\"))"
 ```
 """
 vn_to_string(vn::VarName) = repr((sym = String(getsym(vn)), optic = optic_to_nt(getoptic(vn))))
@@ -861,3 +863,51 @@ function vn_from_string(str)
     new_fields = eval(Meta.parse(str))
     return VarName{Symbol(new_fields.sym)}(nt_to_optic(new_fields.optic))
 end
+
+# -----------------------------------------
+# Alternate implementation with StructTypes
+# -----------------------------------------
+
+optic_to_dict(::typeof(identity)) = Dict(:type => "identity")
+optic_to_dict(::PropertyLens{sym}) where {sym} = Dict(:type => "property", :field => String(sym))
+optic_to_dict(i::IndexLens) = Dict(:type => "index", :indices => index_to_str(i.indices))
+optic_to_dict(c::ComposedOptic) = Dict(:type => "composed", :outer => optic_to_dict(c.outer), :inner => optic_to_dict(c.inner))
+
+function dict_to_optic(dict)
+    # Nested dicts are deserialised to Dict{String, Any}
+    # but the top level dict is deserialised to Dict{Symbol, Any}
+    # so for this recursive function to work we need to first
+    # convert String keys to Symbols
+    dict = Dict(Symbol(k) => v for (k, v) in dict)
+    if dict[:type] == "identity"
+        return identity
+    elseif dict[:type] == "index"
+        return IndexLens(eval(Meta.parse(dict[:indices])))
+    elseif dict[:type] == "property"
+        return PropertyLens{Symbol(dict[:field])}()
+    elseif dict[:type] == "composed"
+        return dict_to_optic(dict[:outer]) ∘ dict_to_optic(dict[:inner])
+    end
+end
+
+struct VarNameWithNTOptic
+    sym::Symbol
+    optic::Dict{Symbol, Any}
+end
+
+function VarNameWithNTOptic(dict::Dict{Symbol, Any})
+    return VarNameWithNTOptic{dict[:sym]}(dict[:optic])
+end
+
+# Serialisation
+StructTypes.StructType(::Type{VarNameWithNTOptic}) = StructTypes.UnorderedStruct()
+
+vn_to_string2(vn::VarName) = JSON3.write(VarNameWithNTOptic(getsym(vn), optic_to_dict(getoptic(vn))))
+
+# Deserialisation
+Base.pairs(vn::VarNameWithNTOptic) = Dict(:sym => vn.sym, :optic => vn.optic)
+
+function vn_from_string2(str)
+    vn_nt = JSON3.read(str, VarNameWithNTOptic)
+    return VarName{vn_nt.sym}(dict_to_optic(vn_nt.optic))
+end
diff --git a/test/varname.jl b/test/varname.jl
index f1d7742..048a48e 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -162,7 +162,7 @@ end
             @varname(y[:], true),
         ]
         for vn in vns
-            @test vn_from_string(vn_to_string(vn)) == vn
+            @test vn_from_string2(vn_to_string2(vn)) == vn
         end
     end
 end

From e38def2b4f1a7eebe166147c2683a558df250045 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 26 Sep 2024 16:14:17 +0100
Subject: [PATCH 09/21] Reduce calls to Meta.parse()

It's only called for ConcretizedSlice now, which could potentially be
removed too.
---
 src/varname.jl | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/varname.jl b/src/varname.jl
index 15782b6..9d7de18 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -868,9 +868,31 @@ end
 # Alternate implementation with StructTypes
 # -----------------------------------------
 
+index_to_dict(i::Integer) = Dict(:type => "integer", :value => i)
+index_to_dict(r::UnitRange) = Dict(:type => "unitrange", :first => first(r), :last => last(r))
+index_to_dict(::Colon) = Dict(:type => "colon")
+index_to_dict(s::ConcretizedSlice{T,R}) where {T,R} = Dict(:type => "concretized_slice", :range => repr(s.range))
+index_to_dict(t::Tuple) = Dict(:type => "tuple", :values => [index_to_dict(x) for x in t])
+
+function dict_to_index(dict)
+    dict = Dict(Symbol(k) => v for (k, v) in dict)
+    if dict[:type] == "integer"
+        return dict[:value]
+    elseif dict[:type] == "unitrange"
+        return dict[:first]:dict[:last]
+    elseif dict[:type] == "colon"
+        return Colon()
+    elseif dict[:type] == "concretized_slice"
+        return ConcretizedSlice(eval(Meta.parse(dict[:range])))
+    elseif dict[:type] == "tuple"
+        return tuple(map(dict_to_index, dict[:values])...)
+    end
+end
+
+
 optic_to_dict(::typeof(identity)) = Dict(:type => "identity")
 optic_to_dict(::PropertyLens{sym}) where {sym} = Dict(:type => "property", :field => String(sym))
-optic_to_dict(i::IndexLens) = Dict(:type => "index", :indices => index_to_str(i.indices))
+optic_to_dict(i::IndexLens) = Dict(:type => "index", :indices => index_to_dict(i.indices))
 optic_to_dict(c::ComposedOptic) = Dict(:type => "composed", :outer => optic_to_dict(c.outer), :inner => optic_to_dict(c.inner))
 
 function dict_to_optic(dict)
@@ -882,7 +904,7 @@ function dict_to_optic(dict)
     if dict[:type] == "identity"
         return identity
     elseif dict[:type] == "index"
-        return IndexLens(eval(Meta.parse(dict[:indices])))
+        return IndexLens(dict_to_index(dict[:indices]))
     elseif dict[:type] == "property"
         return PropertyLens{Symbol(dict[:field])}()
     elseif dict[:type] == "composed"

From a4fbcfdb599061d247821b9006f49f5b7a4e2070 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 26 Sep 2024 16:36:34 +0100
Subject: [PATCH 10/21] Restrict allowed ranges for ConcretizedSlice

---
 src/varname.jl | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/varname.jl b/src/varname.jl
index 9d7de18..62d77be 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -871,10 +871,12 @@ end
 index_to_dict(i::Integer) = Dict(:type => "integer", :value => i)
 index_to_dict(r::UnitRange) = Dict(:type => "unitrange", :first => first(r), :last => last(r))
 index_to_dict(::Colon) = Dict(:type => "colon")
-index_to_dict(s::ConcretizedSlice{T,R}) where {T,R} = Dict(:type => "concretized_slice", :range => repr(s.range))
+index_to_dict(s::ConcretizedSlice{T,Base.OneTo{I}}) where {T,I} = Dict(:type => "concretized_slice", :oneto => s.range.stop)
+index_to_dict(::ConcretizedSlice{T,R}) where {T,R} = error("ConcretizedSlice with range type $(R) not supported")
 index_to_dict(t::Tuple) = Dict(:type => "tuple", :values => [index_to_dict(x) for x in t])
 
 function dict_to_index(dict)
+    # conversion needed because of the same reason as in dict_to_optic
     dict = Dict(Symbol(k) => v for (k, v) in dict)
     if dict[:type] == "integer"
         return dict[:value]
@@ -883,9 +885,11 @@ function dict_to_index(dict)
     elseif dict[:type] == "colon"
         return Colon()
     elseif dict[:type] == "concretized_slice"
-        return ConcretizedSlice(eval(Meta.parse(dict[:range])))
+        return ConcretizedSlice(Base.Slice(Base.OneTo(dict[:oneto])))
     elseif dict[:type] == "tuple"
         return tuple(map(dict_to_index, dict[:values])...)
+    else
+        error("Unknown index type: $(dict[:type])")
     end
 end
 
@@ -909,6 +913,8 @@ function dict_to_optic(dict)
         return PropertyLens{Symbol(dict[:field])}()
     elseif dict[:type] == "composed"
         return dict_to_optic(dict[:outer]) ∘ dict_to_optic(dict[:inner])
+    else
+        error("Unknown optic type: $(dict[:type])")
     end
 end
 

From c3c13fc249427714286165228ba80bd0db1ded91 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 26 Sep 2024 16:38:44 +0100
Subject: [PATCH 11/21] Fix name of wrapper type

---
 src/varname.jl | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/varname.jl b/src/varname.jl
index 62d77be..58a528d 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -918,24 +918,24 @@ function dict_to_optic(dict)
     end
 end
 
-struct VarNameWithNTOptic
+struct VarNameWithDictOptic
     sym::Symbol
     optic::Dict{Symbol, Any}
 end
 
-function VarNameWithNTOptic(dict::Dict{Symbol, Any})
-    return VarNameWithNTOptic{dict[:sym]}(dict[:optic])
+function VarNameWithDictOptic(dict::Dict{Symbol, Any})
+    return VarNameWithDictOptic{dict[:sym]}(dict[:optic])
 end
 
 # Serialisation
-StructTypes.StructType(::Type{VarNameWithNTOptic}) = StructTypes.UnorderedStruct()
+StructTypes.StructType(::Type{VarNameWithDictOptic}) = StructTypes.UnorderedStruct()
 
-vn_to_string2(vn::VarName) = JSON3.write(VarNameWithNTOptic(getsym(vn), optic_to_dict(getoptic(vn))))
+vn_to_string2(vn::VarName) = JSON3.write(VarNameWithDictOptic(getsym(vn), optic_to_dict(getoptic(vn))))
 
 # Deserialisation
-Base.pairs(vn::VarNameWithNTOptic) = Dict(:sym => vn.sym, :optic => vn.optic)
+Base.pairs(vn::VarNameWithDictOptic) = Dict(:sym => vn.sym, :optic => vn.optic)
 
 function vn_from_string2(str)
-    vn_nt = JSON3.read(str, VarNameWithNTOptic)
+    vn_nt = JSON3.read(str, VarNameWithDictOptic)
     return VarName{vn_nt.sym}(dict_to_optic(vn_nt.optic))
 end

From 9ff2bb9c30259307a8a7cbdb93c640a2c0f67e88 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 26 Sep 2024 16:45:16 +0100
Subject: [PATCH 12/21] More tests

---
 test/varname.jl | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/varname.jl b/test/varname.jl
index 048a48e..83be9a4 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -140,6 +140,7 @@ end
 
     @testset "roundtrip conversion to/from string" begin
         y = ones(10)
+        z = ones(5, 2)
         vns = [
             @varname(x),
             @varname(ä),
@@ -160,6 +161,14 @@ end
             @varname(y[end]),
             @varname(y[:], false),
             @varname(y[:], true),
+            @varname(z[:], false),
+            @varname(z[:], true),
+            @varname(z[:][:], false),
+            @varname(z[:][:], true),
+            @varname(z[:,:], false),
+            @varname(z[:,:], true),
+            @varname(z[2:5,:], false),
+            @varname(z[2:5,:], true),
         ]
         for vn in vns
             @test vn_from_string2(vn_to_string2(vn)) == vn

From 3fe838c37c2858e0db5e6ea5e0cce4db37be96f1 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 26 Sep 2024 16:46:58 +0100
Subject: [PATCH 13/21] Remove unneeded extra method for ConcretizedSlice

---
 src/varname.jl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/varname.jl b/src/varname.jl
index 58a528d..974038a 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -457,7 +457,6 @@ struct ConcretizedSlice{T,R} <: AbstractVector{T}
 end
 
 ConcretizedSlice(s::Base.Slice{R}) where {R} = ConcretizedSlice{eltype(s.indices),R}(s.indices)
-ConcretizedSlice(s::Base.OneTo{R}) where {R} = ConcretizedSlice(Base.Slice(s))
 Base.show(io::IO, s::ConcretizedSlice) = print(io, ":")
 Base.show(io::IO, ::MIME"text/plain", s::ConcretizedSlice) =
     print(io, "ConcretizedSlice(", s.range, ")")

From fbfb30615175846e44c16c5c0dc190fe865d4c3d Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 26 Sep 2024 22:30:01 +0100
Subject: [PATCH 14/21] Add StepRange support

---
 src/varname.jl  | 8 +++++---
 test/varname.jl | 1 +
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/varname.jl b/src/varname.jl
index 974038a..425a4a5 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -868,7 +868,8 @@ end
 # -----------------------------------------
 
 index_to_dict(i::Integer) = Dict(:type => "integer", :value => i)
-index_to_dict(r::UnitRange) = Dict(:type => "unitrange", :first => first(r), :last => last(r))
+index_to_dict(r::UnitRange) = Dict(:type => "unitrange", :start => r.start, :stop => r.stop)
+index_to_dict(r::StepRange) = Dict(:type => "steprange", :start => r.start, :stop => r.stop, :step => r.step)
 index_to_dict(::Colon) = Dict(:type => "colon")
 index_to_dict(s::ConcretizedSlice{T,Base.OneTo{I}}) where {T,I} = Dict(:type => "concretized_slice", :oneto => s.range.stop)
 index_to_dict(::ConcretizedSlice{T,R}) where {T,R} = error("ConcretizedSlice with range type $(R) not supported")
@@ -880,7 +881,9 @@ function dict_to_index(dict)
     if dict[:type] == "integer"
         return dict[:value]
     elseif dict[:type] == "unitrange"
-        return dict[:first]:dict[:last]
+        return dict[:start]:dict[:stop]
+    elseif dict[:type] == "steprange"
+        return dict[:start]:dict[:step]:dict[:stop]
     elseif dict[:type] == "colon"
         return Colon()
     elseif dict[:type] == "concretized_slice"
@@ -892,7 +895,6 @@ function dict_to_index(dict)
     end
 end
 
-
 optic_to_dict(::typeof(identity)) = Dict(:type => "identity")
 optic_to_dict(::PropertyLens{sym}) where {sym} = Dict(:type => "property", :field => String(sym))
 optic_to_dict(i::IndexLens) = Dict(:type => "index", :indices => index_to_dict(i.indices))
diff --git a/test/varname.jl b/test/varname.jl
index 83be9a4..9a71dc0 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -150,6 +150,7 @@ end
             @varname(x[1]),
             @varname(var"x[1]"),
             @varname(x[1:10]),
+            @varname(x[1:3:10]),
             @varname(x[1, 2]),
             @varname(x[1, 2:5]),
             @varname(x[:]),

From 73a426b04339cf9d7b56c55122e41558295cf855 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Thu, 26 Sep 2024 22:43:43 +0100
Subject: [PATCH 15/21] Support arrays of integers as indices

---
 src/varname.jl  | 3 +++
 test/varname.jl | 9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/src/varname.jl b/src/varname.jl
index 425a4a5..3261691 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -868,6 +868,7 @@ end
 # -----------------------------------------
 
 index_to_dict(i::Integer) = Dict(:type => "integer", :value => i)
+index_to_dict(v::AbstractVector{Int}) = Dict(:type => "vector", :values => v)
 index_to_dict(r::UnitRange) = Dict(:type => "unitrange", :start => r.start, :stop => r.stop)
 index_to_dict(r::StepRange) = Dict(:type => "steprange", :start => r.start, :stop => r.stop, :step => r.step)
 index_to_dict(::Colon) = Dict(:type => "colon")
@@ -880,6 +881,8 @@ function dict_to_index(dict)
     dict = Dict(Symbol(k) => v for (k, v) in dict)
     if dict[:type] == "integer"
         return dict[:value]
+    elseif dict[:type] == "vector"
+        return collect(Int, dict[:values])
     elseif dict[:type] == "unitrange"
         return dict[:start]:dict[:stop]
     elseif dict[:type] == "steprange"
diff --git a/test/varname.jl b/test/varname.jl
index 9a71dc0..ffa2a6d 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -174,5 +174,14 @@ end
         for vn in vns
             @test vn_from_string2(vn_to_string2(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 = vn_from_string2(vn_to_string2(vn_vec))
+        @test hash(vn_vec) == hash(vn_vec2)
     end
 end

From a5a0aa606f5cd98582bfb150c66b34dab66db3b9 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Fri, 27 Sep 2024 15:54:51 +0100
Subject: [PATCH 16/21] Simplify implementation even more

---
 Project.toml   |  3 +-
 src/varname.jl | 98 ++++++++++++++++++++------------------------------
 2 files changed, 39 insertions(+), 62 deletions(-)

diff --git a/Project.toml b/Project.toml
index 58f1dc5..58e34ff 100644
--- a/Project.toml
+++ b/Project.toml
@@ -9,9 +9,8 @@ version = "0.8.4"
 AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001"
 Accessors = "7d9f7c33-5ae7-4f3b-8dc6-eff91059b697"
 DensityInterface = "b429d917-457f-4dbc-8f4c-0cc954292b1d"
-JSON3 = "0f8b85d8-7281-11e9-16c2-39a750bddbf1"
+JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
-StructTypes = "856f2bd8-1eba-4b0a-8007-ebc267875bd4"
 
 [compat]
 AbstractMCMC = "2, 3, 4, 5"
diff --git a/src/varname.jl b/src/varname.jl
index 3261691..a154d0f 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -1,7 +1,6 @@
 using Accessors
 using Accessors: ComposedOptic, PropertyLens, IndexLens, DynamicIndexLens
-using StructTypes: StructTypes
-using JSON3: JSON3
+using JSON: JSON
 
 const ALLOWED_OPTICS = Union{typeof(identity),PropertyLens,IndexLens,ComposedOptic}
 
@@ -867,79 +866,58 @@ end
 # Alternate implementation with StructTypes
 # -----------------------------------------
 
-index_to_dict(i::Integer) = Dict(:type => "integer", :value => i)
-index_to_dict(v::AbstractVector{Int}) = Dict(:type => "vector", :values => v)
-index_to_dict(r::UnitRange) = Dict(:type => "unitrange", :start => r.start, :stop => r.stop)
-index_to_dict(r::StepRange) = Dict(:type => "steprange", :start => r.start, :stop => r.stop, :step => r.step)
-index_to_dict(::Colon) = Dict(:type => "colon")
-index_to_dict(s::ConcretizedSlice{T,Base.OneTo{I}}) where {T,I} = Dict(:type => "concretized_slice", :oneto => s.range.stop)
+index_to_dict(i::Integer) = Dict("type" => "integer", "value" => i)
+index_to_dict(v::AbstractVector{Int}) = Dict("type" => "vector", "values" => v)
+index_to_dict(r::UnitRange) = Dict("type" => "unitrange", "start" => r.start, "stop" => r.stop)
+index_to_dict(r::StepRange) = Dict("type" => "steprange", "start" => r.start, "stop" => r.stop, "step" => r.step)
+index_to_dict(::Colon) = Dict("type" => "colon")
+index_to_dict(s::ConcretizedSlice{T,Base.OneTo{I}}) where {T,I} = Dict("type" => "concretized_slice", "oneto" => s.range.stop)
 index_to_dict(::ConcretizedSlice{T,R}) where {T,R} = error("ConcretizedSlice with range type $(R) not supported")
-index_to_dict(t::Tuple) = Dict(:type => "tuple", :values => [index_to_dict(x) for x in t])
+index_to_dict(t::Tuple) = Dict("type" => "tuple", "values" => map(index_to_dict, t))
 
 function dict_to_index(dict)
-    # conversion needed because of the same reason as in dict_to_optic
-    dict = Dict(Symbol(k) => v for (k, v) in dict)
-    if dict[:type] == "integer"
-        return dict[:value]
-    elseif dict[:type] == "vector"
-        return collect(Int, dict[:values])
-    elseif dict[:type] == "unitrange"
-        return dict[:start]:dict[:stop]
-    elseif dict[:type] == "steprange"
-        return dict[:start]:dict[:step]:dict[:stop]
-    elseif dict[:type] == "colon"
+    if dict["type"] == "integer"
+        return dict["value"]
+    elseif dict["type"] == "vector"
+        return collect(Int, dict["values"])
+    elseif dict["type"] == "unitrange"
+        return dict["start"]:dict["stop"]
+    elseif dict["type"] == "steprange"
+        return dict["start"]:dict["step"]:dict["stop"]
+    elseif dict["type"] == "colon"
         return Colon()
-    elseif dict[:type] == "concretized_slice"
-        return ConcretizedSlice(Base.Slice(Base.OneTo(dict[:oneto])))
-    elseif dict[:type] == "tuple"
-        return tuple(map(dict_to_index, dict[:values])...)
+    elseif dict["type"] == "concretized_slice"
+        return ConcretizedSlice(Base.Slice(Base.OneTo(dict["oneto"])))
+    elseif dict["type"] == "tuple"
+        return tuple(map(dict_to_index, dict["values"])...)
     else
-        error("Unknown index type: $(dict[:type])")
+        error("Unknown index type: $(dict["type"])")
     end
 end
 
-optic_to_dict(::typeof(identity)) = Dict(:type => "identity")
-optic_to_dict(::PropertyLens{sym}) where {sym} = Dict(:type => "property", :field => String(sym))
-optic_to_dict(i::IndexLens) = Dict(:type => "index", :indices => index_to_dict(i.indices))
-optic_to_dict(c::ComposedOptic) = Dict(:type => "composed", :outer => optic_to_dict(c.outer), :inner => optic_to_dict(c.inner))
+optic_to_dict(::typeof(identity)) = Dict("type" => "identity")
+optic_to_dict(::PropertyLens{sym}) where {sym} = Dict("type" => "property", "field" => String(sym))
+optic_to_dict(i::IndexLens) = Dict("type" => "index", "indices" => index_to_dict(i.indices))
+optic_to_dict(c::ComposedOptic) = Dict("type" => "composed", "outer" => optic_to_dict(c.outer), "inner" => optic_to_dict(c.inner))
 
 function dict_to_optic(dict)
-    # Nested dicts are deserialised to Dict{String, Any}
-    # but the top level dict is deserialised to Dict{Symbol, Any}
-    # so for this recursive function to work we need to first
-    # convert String keys to Symbols
-    dict = Dict(Symbol(k) => v for (k, v) in dict)
-    if dict[:type] == "identity"
+    if dict["type"] == "identity"
         return identity
-    elseif dict[:type] == "index"
-        return IndexLens(dict_to_index(dict[:indices]))
-    elseif dict[:type] == "property"
-        return PropertyLens{Symbol(dict[:field])}()
-    elseif dict[:type] == "composed"
-        return dict_to_optic(dict[:outer]) ∘ dict_to_optic(dict[:inner])
+    elseif dict["type"] == "index"
+        return IndexLens(dict_to_index(dict["indices"]))
+    elseif dict["type"] == "property"
+        return PropertyLens{Symbol(dict["field"])}()
+    elseif dict["type"] == "composed"
+        return dict_to_optic(dict["outer"]) ∘ dict_to_optic(dict["inner"])
     else
-        error("Unknown optic type: $(dict[:type])")
+        error("Unknown optic type: $(dict["type"])")
     end
 end
 
-struct VarNameWithDictOptic
-    sym::Symbol
-    optic::Dict{Symbol, Any}
-end
-
-function VarNameWithDictOptic(dict::Dict{Symbol, Any})
-    return VarNameWithDictOptic{dict[:sym]}(dict[:optic])
-end
+vn_to_dict(vn::VarName) = Dict("sym" => getsym(vn), "optic" => optic_to_dict(getoptic(vn)))
 
-# Serialisation
-StructTypes.StructType(::Type{VarNameWithDictOptic}) = StructTypes.UnorderedStruct()
+dict_to_vn(dict::Dict{<:AbstractString, Any}) = VarName{Symbol(dict["sym"])}(dict_to_optic(dict["optic"]))
 
-vn_to_string2(vn::VarName) = JSON3.write(VarNameWithDictOptic(getsym(vn), optic_to_dict(getoptic(vn))))
+vn_to_string2(vn::VarName) = JSON.json(vn_to_dict(vn))
 
-# Deserialisation
-Base.pairs(vn::VarNameWithDictOptic) = Dict(:sym => vn.sym, :optic => vn.optic)
-
-function vn_from_string2(str)
-    vn_nt = JSON3.read(str, VarNameWithDictOptic)
-    return VarName{vn_nt.sym}(dict_to_optic(vn_nt.optic))
-end
+vn_from_string2(str) = dict_to_vn(JSON.parse(str))

From c1613f9f7e20a723149c64f000d6ccefc76be493 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Fri, 27 Sep 2024 15:55:07 +0100
Subject: [PATCH 17/21] Bump to 0.9.0

---
 Project.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Project.toml b/Project.toml
index 58e34ff..beb0c59 100644
--- a/Project.toml
+++ b/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.8.4"
+version = "0.9.0"
 
 [deps]
 AbstractMCMC = "80f14c24-f653-4e6a-9b94-39d6b0f70001"

From fcd81b0388f8a29f357b44a73900ca8ff8a45ddb Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Fri, 27 Sep 2024 16:03:21 +0100
Subject: [PATCH 18/21] Clean up old code, add docs

---
 src/varname.jl  | 143 ++++++++++--------------------------------------
 test/varname.jl |   4 +-
 2 files changed, 31 insertions(+), 116 deletions(-)

diff --git a/src/varname.jl b/src/varname.jl
index a154d0f..a0a56fa 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -754,118 +754,6 @@ function vsym(expr::Expr)
     end
 end
 
-"""
-    index_to_str(i)
-
-Generates a string representation of the index `i`, or a tuple thereof.
-
-## Examples
-
-```jldoctest
-julia> AbstractPPL.index_to_str(2)
-"2"
-
-julia> AbstractPPL.index_to_str((1, 2:5))
-"(1, 2:5,)"
-
-julia> AbstractPPL.index_to_str(:)
-":"
-
-julia> AbstractPPL.index_to_str(AbstractPPL.ConcretizedSlice(Base.Slice(Base.OneTo(10))))
-"ConcretizedSlice(Base.OneTo(10))"
-```
-"""
-index_to_str(i::Integer) = string(i)
-index_to_str(r::UnitRange) = "$(first(r)):$(last(r))"
-index_to_str(::Colon) = ":"
-index_to_str(s::ConcretizedSlice{T,R}) where {T,R} = "ConcretizedSlice(" * repr(s.range) * ")"
-index_to_str(t::Tuple) = "(" * join(map(index_to_str, t), ", ") * ",)"
-
-"""
-    optic_to_nt(optic)
-
-Convert an optic to a named tuple representation.
-
-## Examples
-```jldoctest; setup=:(using Accessors)
-julia> AbstractPPL.optic_to_nt(identity)
-(type = "identity",)
-
-julia> AbstractPPL.optic_to_nt(@optic _.a)
-(type = "property", field = "a")
-
-julia> AbstractPPL.optic_to_nt(@optic _.a.b)
-(type = "composed", outer = (type = "property", field = "b"), inner = (type = "property", field = "a"))
-
-julia> AbstractPPL.optic_to_nt(@optic _[1])  # uses index_to_str()
-(type = "index", indices = "(1,)")
-```
-"""
-optic_to_nt(::typeof(identity)) = (type = "identity",)
-optic_to_nt(::PropertyLens{sym}) where {sym} = (type = "property", field = String(sym))
-optic_to_nt(i::IndexLens) = (type = "index", indices = index_to_str(i.indices))
-optic_to_nt(c::ComposedOptic) = (type = "composed", outer = optic_to_nt(c.outer), inner = optic_to_nt(c.inner))
-
-
-"""
-    nt_to_optic(nt)
-
-Convert a named tuple representation back to an optic.
-"""
-function nt_to_optic(nt)
-    if nt.type == "identity"
-        return identity
-    elseif nt.type == "index"
-        return IndexLens(eval(Meta.parse(nt.indices)))
-    elseif nt.type == "property"
-        return PropertyLens{Symbol(nt.field)}()
-    elseif nt.type == "composed"
-        return nt_to_optic(nt.outer) ∘ nt_to_optic(nt.inner)
-    end
-end
-
-"""
-    vn_to_string(vn::VarName)
-
-Convert a `VarName` as a string, via an intermediate named tuple. This differs
-from `string(vn)` in that concretised slices are faithfully represented (rather
-than being pretty-printed as colons).
-
-```jldoctest
-julia> vn_to_string(@varname(x))
-"(sym = \\"x\\", optic = (type = \\"identity\\",))"
-
-julia> vn_to_string(@varname(x.a))
-"(sym = \\"x\\", optic = (type = \\"property\\", field = \\"a\\"))"
-
-julia> y = ones(2); vn_to_string(@varname(y[:]))
-"(sym = \\"y\\", optic = (type = \\"index\\", indices = \\"(:,)\\"))"
-
-julia> y = ones(2); vn_to_string(@varname(y[:], true))
-"(sym = \\"y\\", optic = (type = \\"index\\", indices = \\"(ConcretizedSlice(Base.OneTo(2)),)\\"))"
-```
-"""
-vn_to_string(vn::VarName) = repr((sym = String(getsym(vn)), optic = optic_to_nt(getoptic(vn))))
-
-"""
-    vn_from_string(str)
-
-Convert a string representation of a `VarName` back to a `VarName`. The string
-should have been generated by `vn_to_string`.
-
-!!! warning
-    This function should only be used with trusted input, as it uses `eval`
-and `Meta.parse` to parse the string.
-"""
-function vn_from_string(str)
-    new_fields = eval(Meta.parse(str))
-    return VarName{Symbol(new_fields.sym)}(nt_to_optic(new_fields.optic))
-end
-
-# -----------------------------------------
-# Alternate implementation with StructTypes
-# -----------------------------------------
-
 index_to_dict(i::Integer) = Dict("type" => "integer", "value" => i)
 index_to_dict(v::AbstractVector{Int}) = Dict("type" => "vector", "values" => v)
 index_to_dict(r::UnitRange) = Dict("type" => "unitrange", "start" => r.start, "stop" => r.stop)
@@ -918,6 +806,33 @@ vn_to_dict(vn::VarName) = Dict("sym" => getsym(vn), "optic" => optic_to_dict(get
 
 dict_to_vn(dict::Dict{<:AbstractString, Any}) = VarName{Symbol(dict["sym"])}(dict_to_optic(dict["optic"]))
 
-vn_to_string2(vn::VarName) = JSON.json(vn_to_dict(vn))
+"""
+    vn_to_string(vn::VarName)
+
+Convert a `VarName` as a string, via an intermediate dictionary. This differs
+from `string(vn)` in that concretised slices are faithfully represented (rather
+than being pretty-printed as colons).
+
+```jldoctest
+julia> vn_to_string(@varname(x))
+"{\\"optic\\":{\\"type\\":\\"identity\\"},\\"sym\\":\\"x\\"}"
+
+julia> vn_to_string(@varname(x.a))
+"{\\"optic\\":{\\"field\\":\\"a\\",\\"type\\":\\"property\\"},\\"sym\\":\\"x\\"}"
+
+julia> y = ones(2); vn_to_string(@varname(y[:]))
+"{\\"optic\\":{\\"indices\\":{\\"values\\":[{\\"type\\":\\"colon\\"}],\\"type\\":\\"tuple\\"},\\"type\\":\\"index\\"},\\"sym\\":\\"y\\"}"
+
+julia> y = ones(2); vn_to_string(@varname(y[:], true))
+"{\\"optic\\":{\\"indices\\":{\\"values\\":[{\\"oneto\\":2,\\"type\\":\\"concretized_slice\\"}],\\"type\\":\\"tuple\\"},\\"type\\":\\"index\\"},\\"sym\\":\\"y\\"}"
+```
+"""
+vn_to_string(vn::VarName) = JSON.json(vn_to_dict(vn))
+
+"""
+    vn_from_string(str)
 
-vn_from_string2(str) = dict_to_vn(JSON.parse(str))
+Convert a string representation of a `VarName` back to a `VarName`. The string
+should have been generated by `vn_to_string`.
+"""
+vn_from_string(str) = dict_to_vn(JSON.parse(str))
diff --git a/test/varname.jl b/test/varname.jl
index ffa2a6d..94a1d5b 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -172,7 +172,7 @@ end
             @varname(z[2:5,:], true),
         ]
         for vn in vns
-            @test vn_from_string2(vn_to_string2(vn)) == vn
+            @test vn_from_string(vn_to_string(vn)) == vn
         end
 
         # For this VarName, the {de,}serialisation works correctly but we must
@@ -181,7 +181,7 @@ end
         # addresses rather than the contents (thus vn_vec == vn_vec2 returns
         # false).
         vn_vec = @varname(x[[1, 2, 5, 6]])
-        vn_vec2 = vn_from_string2(vn_to_string2(vn_vec))
+        vn_vec2 = vn_from_string(vn_to_string(vn_vec))
         @test hash(vn_vec) == hash(vn_vec2)
     end
 end

From 446dd76d9f2f56f7462e79b88cf047989d955bf3 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Mon, 30 Sep 2024 22:01:13 +0100
Subject: [PATCH 19/21] Allow de/serialisation methods to be extended

---
 docs/src/api.md    |   7 +++
 src/AbstractPPL.jl |  10 ++---
 src/varname.jl     | 105 +++++++++++++++++++++++++++++++++++----------
 test/varname.jl    |  18 +++++++-
 4 files changed, 112 insertions(+), 28 deletions(-)

diff --git a/docs/src/api.md b/docs/src/api.md
index 3507419..56559d7 100644
--- a/docs/src/api.md
+++ b/docs/src/api.md
@@ -12,6 +12,13 @@ subsumedby
 vsym
 @varname
 @vsym
+```
+
+## VarName serialisation
+
+```@docs
+index_to_dict
+dict_to_index
 vn_to_string
 vn_from_string
 ```
diff --git a/src/AbstractPPL.jl b/src/AbstractPPL.jl
index 9a4a768..2def523 100644
--- a/src/AbstractPPL.jl
+++ b/src/AbstractPPL.jl
@@ -8,13 +8,13 @@ export VarName,
     subsumes,
     subsumedby,
     varname,
-    vn_to_string,
-    vn_from_string,
-    vn_to_string2,
-    vn_from_string2,
     vsym,
     @varname,
-    @vsym
+    @vsym,
+    index_to_dict,
+    dict_to_index,
+    vn_to_string,
+    vn_from_string
 
 
 # Abstract model functions
diff --git a/src/varname.jl b/src/varname.jl
index a0a56fa..2086cb8 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -754,32 +754,87 @@ function vsym(expr::Expr)
     end
 end
 
-index_to_dict(i::Integer) = Dict("type" => "integer", "value" => i)
-index_to_dict(v::AbstractVector{Int}) = Dict("type" => "vector", "values" => v)
-index_to_dict(r::UnitRange) = Dict("type" => "unitrange", "start" => r.start, "stop" => r.stop)
-index_to_dict(r::StepRange) = Dict("type" => "steprange", "start" => r.start, "stop" => r.stop, "step" => r.step)
-index_to_dict(::Colon) = Dict("type" => "colon")
-index_to_dict(s::ConcretizedSlice{T,Base.OneTo{I}}) where {T,I} = Dict("type" => "concretized_slice", "oneto" => s.range.stop)
-index_to_dict(::ConcretizedSlice{T,R}) where {T,R} = error("ConcretizedSlice with range type $(R) not supported")
-index_to_dict(t::Tuple) = Dict("type" => "tuple", "values" => map(index_to_dict, t))
-
+# String constants for each index type that we support serialisation /
+# deserialisation of
+const _BASE_INTEGER_TYPE = "Base.Integer"
+const _BASE_VECTOR_TYPE = "Base.Vector"
+const _BASE_UNITRANGE_TYPE = "Base.UnitRange"
+const _BASE_STEPRANGE_TYPE = "Base.StepRange"
+const _BASE_ONETO_TYPE = "Base.OneTo"
+const _BASE_COLON_TYPE = "Base.Colon"
+const _CONCRETIZED_SLICE_TYPE = "AbstractPPL.ConcretizedSlice"
+const _BASE_TUPLE_TYPE = "Base.Tuple"
+
+"""
+    index_to_dict(::Integer)
+    index_to_dict(::AbstractVector{Int})
+    index_to_dict(::UnitRange)
+    index_to_dict(::StepRange)
+    index_to_dict(::Colon)
+    index_to_dict(::ConcretizedSlice{T, Base.OneTo{I}}) where {T, I}
+    index_to_dict(::Tuple)
+
+Convert an index `i` to a dictionary representation.
+"""
+index_to_dict(i::Integer) = Dict("type" => _BASE_INTEGER_TYPE, "value" => i)
+index_to_dict(v::Vector{Int}) = Dict("type" => _BASE_VECTOR_TYPE, "values" => v)
+index_to_dict(r::UnitRange) = Dict("type" => _BASE_UNITRANGE_TYPE, "start" => r.start, "stop" => r.stop)
+index_to_dict(r::StepRange) = Dict("type" => _BASE_STEPRANGE_TYPE, "start" => r.start, "stop" => r.stop, "step" => r.step)
+index_to_dict(r::Base.OneTo{I}) where {I} = Dict("type" => _BASE_ONETO_TYPE, "stop" => r.stop)
+index_to_dict(::Colon) = Dict("type" => _BASE_COLON_TYPE)
+index_to_dict(s::ConcretizedSlice{T,R}) where {T,R} = Dict("type" => _CONCRETIZED_SLICE_TYPE, "range" => index_to_dict(s.range))
+index_to_dict(t::Tuple) = Dict("type" => _BASE_TUPLE_TYPE, "values" => map(index_to_dict, t))
+
+"""
+    dict_to_index(dict)
+    dict_to_index(symbol_val, dict)
+
+Convert a dictionary representation of an index `dict` to an index.
+
+Users can extend the functionality of `dict_to_index` (and hence `VarName`
+de/serialisation) by extending this method along with [`index_to_dict`](@ref).
+Specifically, suppose you have a custom index type `MyIndexType` and you want
+to be able to de/serialise a `VarName` containing this index type. You should
+then implement the following two methods:
+
+1. `AbstractPPL.index_to_dict(i::MyIndexType)` should return a dictionary
+   representation of the index `i`. This dictionary must contain the key
+   `"type"`, and the corresponding value must be a string that uniquely
+   identifies the index type. Generally, it makes sense to use the name of the
+   type (perhaps prefixed with module qualifiers) as this value to avoid
+   clashes. The remainder of the dictionary can have any structure you like.
+
+2. Suppose the value of `index_to_dict(i)["type"]` is "MyModule.MyIndexType".
+   You should then implement the corresponding method
+   `AbstractPPL.dict_to_index(::Val{Symbol("MyModule.MyIndexType")}, dict)`,
+   which should take the dictionary representation as the second argument and
+   return the original `MyIndexType` object.
+
+To see an example of this in action, you can look in the the AbstractPPL test
+suite, which contains a test for serialising OffsetArrays.
+"""
 function dict_to_index(dict)
-    if dict["type"] == "integer"
+    t = dict["type"]
+    if t == _BASE_INTEGER_TYPE
         return dict["value"]
-    elseif dict["type"] == "vector"
+    elseif t == _BASE_VECTOR_TYPE
         return collect(Int, dict["values"])
-    elseif dict["type"] == "unitrange"
+    elseif t == _BASE_UNITRANGE_TYPE
         return dict["start"]:dict["stop"]
-    elseif dict["type"] == "steprange"
+    elseif t == _BASE_STEPRANGE_TYPE
         return dict["start"]:dict["step"]:dict["stop"]
-    elseif dict["type"] == "colon"
+    elseif t == _BASE_ONETO_TYPE
+        return Base.OneTo(dict["stop"])
+    elseif t == _BASE_COLON_TYPE
         return Colon()
-    elseif dict["type"] == "concretized_slice"
-        return ConcretizedSlice(Base.Slice(Base.OneTo(dict["oneto"])))
-    elseif dict["type"] == "tuple"
+    elseif t == _CONCRETIZED_SLICE_TYPE
+        return ConcretizedSlice(Base.Slice(dict_to_index(dict["range"])))
+    elseif t == _BASE_TUPLE_TYPE
         return tuple(map(dict_to_index, dict["values"])...)
     else
-        error("Unknown index type: $(dict["type"])")
+        # Will error if the method is not defined, but this hook allows users
+        # to extend this function
+        return dict_to_index(Val(Symbol(t)), dict)
     end
 end
 
@@ -813,6 +868,12 @@ Convert a `VarName` as a string, via an intermediate dictionary. This differs
 from `string(vn)` in that concretised slices are faithfully represented (rather
 than being pretty-printed as colons).
 
+For `VarName`s which index into an array, this function will only work if the
+indices can be serialised. This is true for all standard Julia index types, but
+if you are using custom index types, you will need to implement the
+`index_to_dict` and `dict_to_index` methods for those types. See the
+documentation of [`dict_to_index`](@ref) for instructions on how to do this.
+
 ```jldoctest
 julia> vn_to_string(@varname(x))
 "{\\"optic\\":{\\"type\\":\\"identity\\"},\\"sym\\":\\"x\\"}"
@@ -821,18 +882,18 @@ julia> vn_to_string(@varname(x.a))
 "{\\"optic\\":{\\"field\\":\\"a\\",\\"type\\":\\"property\\"},\\"sym\\":\\"x\\"}"
 
 julia> y = ones(2); vn_to_string(@varname(y[:]))
-"{\\"optic\\":{\\"indices\\":{\\"values\\":[{\\"type\\":\\"colon\\"}],\\"type\\":\\"tuple\\"},\\"type\\":\\"index\\"},\\"sym\\":\\"y\\"}"
+"{\\"optic\\":{\\"indices\\":{\\"values\\":[{\\"type\\":\\"Base.Colon\\"}],\\"type\\":\\"Base.Tuple\\"},\\"type\\":\\"index\\"},\\"sym\\":\\"y\\"}"
 
 julia> y = ones(2); vn_to_string(@varname(y[:], true))
-"{\\"optic\\":{\\"indices\\":{\\"values\\":[{\\"oneto\\":2,\\"type\\":\\"concretized_slice\\"}],\\"type\\":\\"tuple\\"},\\"type\\":\\"index\\"},\\"sym\\":\\"y\\"}"
+"{\\"optic\\":{\\"indices\\":{\\"values\\":[{\\"range\\":{\\"stop\\":2,\\"type\\":\\"Base.OneTo\\"},\\"type\\":\\"AbstractPPL.ConcretizedSlice\\"}],\\"type\\":\\"Base.Tuple\\"},\\"type\\":\\"index\\"},\\"sym\\":\\"y\\"}"
 ```
 """
 vn_to_string(vn::VarName) = JSON.json(vn_to_dict(vn))
 
 """
-    vn_from_string(str)
+    vn_from_string(str::AbstractString)
 
 Convert a string representation of a `VarName` back to a `VarName`. The string
 should have been generated by `vn_to_string`.
 """
-vn_from_string(str) = dict_to_vn(JSON.parse(str))
+vn_from_string(str::AbstractString) = dict_to_vn(JSON.parse(str))
diff --git a/test/varname.jl b/test/varname.jl
index 94a1d5b..3782bf3 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -138,7 +138,7 @@ end
         @inferred Accessors.set(c, @varname(b.a[1]), 10)
     end
 
-    @testset "roundtrip conversion to/from string" begin
+    @testset "de/serialisation of VarNames" begin
         y = ones(10)
         z = ones(5, 2)
         vns = [
@@ -184,4 +184,20 @@ end
         vn_vec2 = vn_from_string(vn_to_string(vn_vec))
         @test hash(vn_vec) == hash(vn_vec2)
     end
+
+    @testset "de/serialisation of VarNames with custom index types" begin
+        using OffsetArrays: OffsetArrays, Origin
+        weird = Origin(4)(ones(10))
+        vn = @varname(weird[:], true)
+
+        # This won't work as we don't yet know how to handle OffsetArray
+        @test_throws MethodError vn_to_string(vn)
+
+        # Now define the relevant methods
+        AbstractPPL.index_to_dict(o::OffsetArrays.IdOffsetRange{I, R}) where {I,R} = Dict("type" => "OffsetArrays.OffsetArray", "parent" => AbstractPPL.index_to_dict(o.parent), "offset" => o.offset)
+        AbstractPPL.dict_to_index(::Val{Symbol("OffsetArrays.OffsetArray")}, d) = OffsetArrays.IdOffsetRange(AbstractPPL.dict_to_index(d["parent"]), d["offset"])
+
+        # Serialisation should now work
+        @test vn_from_string(vn_to_string(vn)) == vn
+    end
 end

From 15ee51696157d5dd42ea6c06b2146ce34c084930 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Mon, 30 Sep 2024 22:11:10 +0100
Subject: [PATCH 20/21] Update docs

---
 src/varname.jl | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/varname.jl b/src/varname.jl
index 2086cb8..118a420 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -797,14 +797,14 @@ Specifically, suppose you have a custom index type `MyIndexType` and you want
 to be able to de/serialise a `VarName` containing this index type. You should
 then implement the following two methods:
 
-1. `AbstractPPL.index_to_dict(i::MyIndexType)` should return a dictionary
-   representation of the index `i`. This dictionary must contain the key
-   `"type"`, and the corresponding value must be a string that uniquely
+1. `AbstractPPL.index_to_dict(i::MyModule.MyIndexType)` should return a
+   dictionary representation of the index `i`. This dictionary must contain the
+   key `"type"`, and the corresponding value must be a string that uniquely
    identifies the index type. Generally, it makes sense to use the name of the
    type (perhaps prefixed with module qualifiers) as this value to avoid
    clashes. The remainder of the dictionary can have any structure you like.
 
-2. Suppose the value of `index_to_dict(i)["type"]` is "MyModule.MyIndexType".
+2. Suppose the value of `index_to_dict(i)["type"]` is `"MyModule.MyIndexType"`.
    You should then implement the corresponding method
    `AbstractPPL.dict_to_index(::Val{Symbol("MyModule.MyIndexType")}, dict)`,
    which should take the dictionary representation as the second argument and

From e0e5322e9bcea419b70d96088431773cedd259c6 Mon Sep 17 00:00:00 2001
From: Penelope Yong <penelopeysm@gmail.com>
Date: Mon, 30 Sep 2024 22:12:46 +0100
Subject: [PATCH 21/21] Name functions more consistently

---
 docs/src/api.md    |  4 ++--
 src/AbstractPPL.jl |  4 ++--
 src/varname.jl     | 22 +++++++++++-----------
 test/varname.jl    |  8 ++++----
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/docs/src/api.md b/docs/src/api.md
index 56559d7..0e7f3f8 100644
--- a/docs/src/api.md
+++ b/docs/src/api.md
@@ -19,8 +19,8 @@ vsym
 ```@docs
 index_to_dict
 dict_to_index
-vn_to_string
-vn_from_string
+varname_to_string
+string_to_varname
 ```
 
 ## Abstract model functions
diff --git a/src/AbstractPPL.jl b/src/AbstractPPL.jl
index 2def523..33fa417 100644
--- a/src/AbstractPPL.jl
+++ b/src/AbstractPPL.jl
@@ -13,8 +13,8 @@ export VarName,
     @vsym,
     index_to_dict,
     dict_to_index,
-    vn_to_string,
-    vn_from_string
+    varname_to_string,
+    string_to_varname
 
 
 # Abstract model functions
diff --git a/src/varname.jl b/src/varname.jl
index 118a420..48d7c5c 100644
--- a/src/varname.jl
+++ b/src/varname.jl
@@ -857,12 +857,12 @@ function dict_to_optic(dict)
     end
 end
 
-vn_to_dict(vn::VarName) = Dict("sym" => getsym(vn), "optic" => optic_to_dict(getoptic(vn)))
+varname_to_dict(vn::VarName) = Dict("sym" => getsym(vn), "optic" => optic_to_dict(getoptic(vn)))
 
-dict_to_vn(dict::Dict{<:AbstractString, Any}) = VarName{Symbol(dict["sym"])}(dict_to_optic(dict["optic"]))
+dict_to_varname(dict::Dict{<:AbstractString, Any}) = VarName{Symbol(dict["sym"])}(dict_to_optic(dict["optic"]))
 
 """
-    vn_to_string(vn::VarName)
+    varname_to_string(vn::VarName)
 
 Convert a `VarName` as a string, via an intermediate dictionary. This differs
 from `string(vn)` in that concretised slices are faithfully represented (rather
@@ -875,25 +875,25 @@ if you are using custom index types, you will need to implement the
 documentation of [`dict_to_index`](@ref) for instructions on how to do this.
 
 ```jldoctest
-julia> vn_to_string(@varname(x))
+julia> varname_to_string(@varname(x))
 "{\\"optic\\":{\\"type\\":\\"identity\\"},\\"sym\\":\\"x\\"}"
 
-julia> vn_to_string(@varname(x.a))
+julia> varname_to_string(@varname(x.a))
 "{\\"optic\\":{\\"field\\":\\"a\\",\\"type\\":\\"property\\"},\\"sym\\":\\"x\\"}"
 
-julia> y = ones(2); vn_to_string(@varname(y[:]))
+julia> y = ones(2); varname_to_string(@varname(y[:]))
 "{\\"optic\\":{\\"indices\\":{\\"values\\":[{\\"type\\":\\"Base.Colon\\"}],\\"type\\":\\"Base.Tuple\\"},\\"type\\":\\"index\\"},\\"sym\\":\\"y\\"}"
 
-julia> y = ones(2); vn_to_string(@varname(y[:], true))
+julia> y = ones(2); varname_to_string(@varname(y[:], true))
 "{\\"optic\\":{\\"indices\\":{\\"values\\":[{\\"range\\":{\\"stop\\":2,\\"type\\":\\"Base.OneTo\\"},\\"type\\":\\"AbstractPPL.ConcretizedSlice\\"}],\\"type\\":\\"Base.Tuple\\"},\\"type\\":\\"index\\"},\\"sym\\":\\"y\\"}"
 ```
 """
-vn_to_string(vn::VarName) = JSON.json(vn_to_dict(vn))
+varname_to_string(vn::VarName) = JSON.json(varname_to_dict(vn))
 
 """
-    vn_from_string(str::AbstractString)
+    string_to_varname(str::AbstractString)
 
 Convert a string representation of a `VarName` back to a `VarName`. The string
-should have been generated by `vn_to_string`.
+should have been generated by `varname_to_string`.
 """
-vn_from_string(str::AbstractString) = dict_to_vn(JSON.parse(str))
+string_to_varname(str::AbstractString) = dict_to_varname(JSON.parse(str))
diff --git a/test/varname.jl b/test/varname.jl
index 3782bf3..7a92d1e 100644
--- a/test/varname.jl
+++ b/test/varname.jl
@@ -172,7 +172,7 @@ end
             @varname(z[2:5,:], true),
         ]
         for vn in vns
-            @test vn_from_string(vn_to_string(vn)) == vn
+            @test string_to_varname(varname_to_string(vn)) == vn
         end
 
         # For this VarName, the {de,}serialisation works correctly but we must
@@ -181,7 +181,7 @@ end
         # addresses rather than the contents (thus vn_vec == vn_vec2 returns
         # false).
         vn_vec = @varname(x[[1, 2, 5, 6]])
-        vn_vec2 = vn_from_string(vn_to_string(vn_vec))
+        vn_vec2 = string_to_varname(varname_to_string(vn_vec))
         @test hash(vn_vec) == hash(vn_vec2)
     end
 
@@ -191,13 +191,13 @@ end
         vn = @varname(weird[:], true)
 
         # This won't work as we don't yet know how to handle OffsetArray
-        @test_throws MethodError vn_to_string(vn)
+        @test_throws MethodError varname_to_string(vn)
 
         # Now define the relevant methods
         AbstractPPL.index_to_dict(o::OffsetArrays.IdOffsetRange{I, R}) where {I,R} = Dict("type" => "OffsetArrays.OffsetArray", "parent" => AbstractPPL.index_to_dict(o.parent), "offset" => o.offset)
         AbstractPPL.dict_to_index(::Val{Symbol("OffsetArrays.OffsetArray")}, d) = OffsetArrays.IdOffsetRange(AbstractPPL.dict_to_index(d["parent"]), d["offset"])
 
         # Serialisation should now work
-        @test vn_from_string(vn_to_string(vn)) == vn
+        @test string_to_varname(varname_to_string(vn)) == vn
     end
 end