ConstructionBase integration (#99)

* WIP

* constructorof is in outer constructors expr block now

* make sure constructionbase also does not need to be in scope

* implement methods for 1.6 compat

* Apply suggestions from code review

Co-authored-by: Curtis Vogt <[email protected]>

* permalink

* compat bounds

* bump version (patch)

---------

Co-authored-by: Curtis Vogt <[email protected]>

Project.toml

@@ -1,25 +1,31 @@
 name = "Legolas"
 uuid = "741b9549-f6ed-4911-9fbf-4a1c0c97f0cd"
 authors = ["Beacon Biosignals, Inc."]
-version = "0.5.13"
+version = "0.5.14"
 
 [deps]
 Arrow = "69666777-d1a9-59fb-9406-91d4454c9d45"
+ConstructionBase = "187b0558-2788-49d3-abe0-74a17ed4e7c9"
 Tables = "bd369af6-aec1-5ad0-b16a-f7cc5008161c"
 UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
 
 [compat]
+Accessors = "0.1"
+Aqua = "0.6"
 Arrow = "2"
 Compat = "3.34, 4"
+ConstructionBase = "1.5"
 DataFrames = "1"
 Tables = "1.4"
 julia = "1.6"
 
 [extras]
+Accessors = "7d9f7c33-5ae7-4f3b-8dc6-eff91059b697"
+Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
 Compat = "34da2185-b29b-5c13-b0c7-acf172513d20"
 DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
 
 [targets]
-test = ["Compat", "DataFrames", "Test", "UUIDs"]
+test = ["Accessors", "Aqua", "Compat", "DataFrames", "Test", "UUIDs"]

src/Legolas.jl

@@ -1,6 +1,7 @@
 module Legolas
 
 using Tables, Arrow, UUIDs
+using ConstructionBase: ConstructionBase
 
 const LEGOLAS_SCHEMA_QUALIFIED_METADATA_KEY = "legolas_schema_qualified"
 

src/schemas.jl

@@ -344,6 +344,24 @@ abstract type AbstractRecord <: Tables.AbstractRow end
 @inline Tables.columnnames(r::AbstractRecord) = fieldnames(typeof(r))
 @inline Tables.schema(::AbstractVector{R}) where {R<:AbstractRecord} = Tables.Schema(fieldnames(R), fieldtypes(R))
 
+# we need a bit of extra work to integrate with ConstructionBase for pre-1.7 due
+# to the overload of `propertynames(::Tables.AbstractRow)`
+#
+# we _could_ overload `check_properties_are_fields` but that's no part of the
+# public API so this is safer
+@static if VERSION < v"1.7"
+    ConstructionBase.getproperties(r::AbstractRecord) = NamedTuple(r)
+    # largely copy-paste from ConstructionBase.setproperties_object:
+    # https://github.com/JuliaObjects/ConstructionBase.jl/blob/cd24e541fd90ab54d2ee12ddd6ccd229be9a5f1e/src/ConstructionBase.jl#L211-L218
+    function ConstructionBase.setproperties(r::R, patch::NamedTuple) where {R <: AbstractRecord}
+        nt = ConstructionBase.getproperties(r)
+        nt_new = merge(nt, patch)
+        ConstructionBase.check_patch_properties_exist(nt_new, nt, r, patch)
+        args = Tuple(nt_new) # old julia inference prefers if we wrap in Tuple
+        return ConstructionBase.constructorof(R)(args...)
+    end
+end
+
 """
     Legolas.schema_version_from_record(record::Legolas.AbstractRecord)
 
@@ -607,7 +625,13 @@ function _generate_record_type_definitions(schema_version::SchemaVersion, record
     # generate `inner_constructor_definitions` and `outer_constructor_definitions`
     R = record_type_symbol
     kwargs_from_row = [Expr(:kw, n, :(get(row, $(Base.Meta.quot(n)), missing))) for n in keys(record_fields)]
-    outer_constructor_definitions = :($R(row) = $R(; $(kwargs_from_row...)))
+    outer_constructor_definitions = quote
+        $R(row) = $R(; $(kwargs_from_row...))
+        function $ConstructionBase.constructorof(::Type{<:$R})
+            nt = NamedTuple{$((:($k) for k in keys(record_fields))..., )}
+            (args...) -> $R(nt(args))
+        end
+    end
     if isempty(type_param_defs)
         inner_constructor_definitions = quote
             function $R(; $(field_kwargs...))

test/runtests.jl

@@ -1,6 +1,8 @@
 using Compat: current_exceptions
 using Legolas, Test, DataFrames, Arrow, UUIDs
 using Legolas: SchemaVersion, @schema, @version, SchemaVersionDeclarationError, DeclaredFieldInfo
+using Accessors
+using Aqua
 
 @test_throws SchemaVersionDeclarationError("no prior `@schema` declaration found in current module") @version(TestV1, begin x end)
 
@@ -9,6 +11,12 @@ module Tour
     include(joinpath(dirname(@__DIR__), "examples", "tour.jl"))
 end
 
+@testset "aqua" begin
+    # picks up ambiguities in dependencies, and one hit for unbound type
+    # parameters for `accepted_field_type(::SchemaVersion, ::Type{Union{Missing,T}})`
+    Aqua.test_all(Legolas; ambiguities=false, unbound_args=false)
+end
+
 @testset "Legolas.lift" begin
     @test ismissing(Legolas.lift(sin, nothing))
     @test ismissing(Legolas.lift(sin, missing))
@@ -185,6 +193,7 @@ end
 module Namespace91
     using Test
     using Legolas: @schema, @version, tobuffer, read
+    using Accessors: @set
 
     # Define something else with the name Legolas
     struct Legolas end
@@ -203,6 +212,10 @@ module Namespace91
         @test A91V1(; a=1) == A91V1(; a=1)
         @test isequal(A91V1(; a=1), A91V1(; a=1))
         @test read(tobuffer([A91V1(; a=1)], A91V1SchemaVersion())).a == [1]
+
+        a = A91V1(; a=1)
+        a2 = @set a.a = 2
+        @test a2.a == 2
     end
 end # module
 
@@ -686,3 +699,32 @@ end
         @test_throws e FieldErrorV3(; a="foo bar")
     end
 end
+
+#####
+##### ConstructionBase/Accessors integration
+#####
+
+@testset "ConstructionBase/Accessors integration" begin
+    p = ParentV1(; x=[1,2,3], y="hello")
+    p2 = @set p.y = "okay"
+    @test p2 isa ParentV1
+    @test p2.y == "okay"
+    @test p2 !== p
+    @test p2.x === p.x
+    @test p2.y != p.y
+
+    e = ArgumentError("Invalid value set for field `y`, expected AbstractString, got a value of type Int64 (1)")
+    @test_throws e @set p.y = 1
+
+    n = NestedV1(; gc=GrandchildV1(; x=[1,2], y="hello", z=nothing, a=1), k=:okay)
+    n2 = @set n.gc.z = "yay!"
+    @test n2.gc.z == "yay!"
+    @test n2.gc != n.gc
+    # parametric on :k
+    @test typeof(n2) == typeof(n)
+    @test n2.k === n.k
+
+    n3 = @set n.k = missing
+    @test n3 isa NestedV1{Missing}
+    @test ismissing(n3.k)
+end