fix: correct Product constructor (#13)

test: add test for Product and NoMetadata

Author: Beforerr

src/dataset.jl

@@ -72,9 +72,5 @@ function DataSet(ld::LDataSet; kwargs...)
 end
 
 Base.parent(ds::AbstractDataSet) = ds.data
-for f in (:getindex, :iterate, :size, :length, :keys)
-    @eval Base.$f(var::AbstractDataSet, args...) = $f(parent(var), args...)
-end
-
 Base.getindex(ds::DataSet, i::Integer) = _nth(values(ds.data), i)
 Base.push!(ds::DataSet, v) = push!(ds.data, v)

src/metadata.jl

@@ -13,16 +13,14 @@ struct NoMetadata end
 const NoData = NoMetadata
 
 Base.keys(::NoMetadata) = ()
+Base.values(::NoMetadata) = ()
+Base.iterate(::NoMetadata) = nothing
 
 # Allow merging NoMetadata with a Dict or keyword arguments
-Base.merge(::NoMetadata, d::AbstractDict) = copy(d)
-Base.merge(::NoMetadata, p::Base.Pairs) = Dict{Any,Any}(p)
+Base.merge(::NoMetadata, d::AbstractDict) = length(d) == 0 ? NoMetadata() : copy(d)
+Base.merge(::NoMetadata, p::Base.Pairs) = length(p) == 0 ? NoMetadata() : p
 Base.merge(m::NoMetadata, d, rest...) = merge(merge(m, d), rest...)
 
 Base.haskey(::NoMetadata, args...) = false
 Base.get(::NoMetadata, key, default=nothing) = default
-Base.length(::NoMetadata) = 0
-for f in (:NamedTuple, :Dict)
-    @eval Base.$f(::NoMetadata) = $f()
-    @eval Base.convert(::Type{$f}, ::NoMetadata) = $f()
-end
+Base.length(::NoMetadata) = 0

src/product.jl

@@ -1,23 +1,23 @@
+# References:
+# https://github.com/MakieOrg/AlgebraOfGraphics.jl/blob/master/src/algebra/layer.jl
+
 struct Product{A,F,MD} <: AbstractProduct
     data::A
     transformation::F
     name::Union{String,Symbol}
     metadata::MD
-    function Product(data::A, transformation::F, name="", metadata::MD=NoMetadata(); kwargs...) where {A,F,MD}
-        metadata = merge(metadata, kwargs)
-        new{A,F,MD}(data, transformation, name, metadata)
-    end
 end
 
-function Product(data; transformation=identity, name="", metadata=NoMetadata(), kwargs...)
-    Product(data, transformation, name, metadata; kwargs...)
+function Product(data::A, transformation=identity; name="", metadata=NoMetadata(), kwargs...) where {A}
+    new_meta = merge(metadata, kwargs)
+    Product(data, transformation, name, new_meta)
 end
 
-data(p::Product) = p.data
-func(p::AbstractProduct) = identity
+Base.parent(p::Product) = p.data
+func(::AbstractProduct) = identity
 func(p::Product) = p.transformation
 
-(p::AbstractProduct)(args...; kwargs...) = func(p)(data(p), args...; kwargs...)
+(p::AbstractProduct)(args...; kwargs...) = func(p)(parent(p), args...; kwargs...)
 
 """Create a new product with the composed function"""
 ∘(f, p::AbstractProduct) = @set p.transformation = f ∘ func(p)
@@ -26,22 +26,19 @@ func(p::Product) = p.transformation
 # Allow chaining of transformations with multiple products
 ∘(g::AbstractProduct, f::AbstractProduct) = @set g.transformation = func(g) ∘ func(f)
 
-function set(ds::AbstractProduct, args...; name=nothing, data=nothing, kwargs...)
-    !isnothing(name) && (ds = @set ds.name = name)
-    !isnothing(data) && (ds = @set ds.data = data)
-    (!isnothing(kwargs) || !isnothing(args)) && (ds = @set ds.metadata = set(ds.metadata, args...; kwargs...))
-    return ds
+function set(p::Product; name=nothing, data=nothing, transformation=nothing, metadata=nothing, kwargs...)
+    !isnothing(name) && (p = @set p.name = name)
+    !isnothing(data) && (p = @set p.data = data)
+    !isnothing(transformation) && (p = @set p.transformation = transformation)
+    !isnothing(metadata) && (p = @set p.metadata = metadata)
+    length(kwargs) > 0 && (p = @set p.metadata = merge(p.metadata, kwargs))
+    return p
 end
 
-function set!!(ds::Product, args...; name=nothing, data=nothing, kwargs...)
-    !isnothing(name) && (ds = @set ds.name = name)
-    !isnothing(data) && (ds = @set ds.data = data)
-    set!(ds.metadata, args...; kwargs...)
-    ds
-end
+set(p::Product, data, transformation=nothing; kwargs...) = set(p; data, transformation, kwargs...)
 
 function Base.show(io::IO, p::Product)
     n = name(p)
-    isempty(n) ? print(io, data(p)) : print(io, n)
+    isempty(n) ? print(io, parent(p)) : print(io, n)
     func(p) !== identity && print(io, " [", func(p), "]")
 end

src/types.jl

@@ -57,6 +57,10 @@ Base.push!(p::Union{Project,Instrument}, v) = insert!(p, name(v), v)
 Base.get(var::AbstractModel, s, d=nothing) = get(meta(var), s, d)
 Base.get(f::Function, var::AbstractModel, s) = get(f, meta(var), s)
 
+for f in (:getindex, :iterate, :size, :length, :keys, :values)
+    @eval Base.$f(var::AbstractProduct, args...) = $f(parent(var), args...)
+end
+
 # https://github.com/rafaqz/DimensionalData.jl/blob/main/src/array/show.jl
 
 Base.show(io::IO, p::T) where {T<:AbstractModel} = print(io, name(p))

src/utils.jl

@@ -89,17 +89,6 @@ function rename!(d::Dict, old_keys::Union{Tuple,Vector}, new_key)
     end
 end
 
-function set!(d::Dict, args::Pair...; kwargs...)
-    foreach(args) do (k, v)
-        d[k] = v
-    end
-    merge!(d, kwargs)
-end
-
-function set(d::Dict, args::Pair...; kwargs...)
-    return merge(d, Dict(args...), kwargs)
-end
-
 # https://github.com/rafaqz/DimensionalData.jl/blob/main/src/Dimensions/show.jl#L5
 function colors(i)
     colors = [209, 32, 81, 204, 249, 166, 37]

test/metadata.jl

@@ -0,0 +1,53 @@
+@testitem "NoMetadata" begin
+    using SpaceDataModel: NoMetadata
+    nm = NoMetadata()
+
+    # Test keys operation
+    @test keys(nm) == () == keys(NamedTuple())
+    @test length(keys(nm)) == 0
+    @info values(nm)
+
+    # Test haskey operation
+    @test haskey(nm, "any_key") == false
+    @test haskey(nm, :symbol_key) == false
+    # Test get operation with default
+    @test get(nm, "key", nothing) == nothing
+end
+
+@testitem "NoMetadata Merging Operations" begin
+    using SpaceDataModel: NoMetadata
+
+    nm = NoMetadata()
+
+    # Test merging with Dict
+    dict1 = Dict("key1" => "value1", "key2" => "value2")
+    merged1 = merge(nm, dict1)
+    @test merged1 == dict1
+    @test merged1 !== dict1  # Should be a copy
+    @test merge(nm, Dict()) == nm # Test merging with empty Dict
+end
+
+@testitem "NoMetadata Type Conversions" begin
+    using SpaceDataModel: NoMetadata
+    nm = NoMetadata()
+    # Test conversion to NamedTuple
+    @test NamedTuple(nm) == (;)
+    # Test conversion to Dict
+    @test Dict(pairs(nm)) == Dict()
+end
+
+@testitem "NoMetadata in Product Context" begin
+    using SpaceDataModel: NoMetadata, Product
+
+    # Test that NoMetadata works correctly in Product constructor
+    nm = NoMetadata()
+
+    # When kwargs are provided, NoMetadata gets merged into Dict
+    p2 = Product([1, 2, 3]; metadata=nm, extra_key="value")
+    @test p2.metadata isa AbstractDict
+    @test p2.metadata[:extra_key] == "value"
+
+    # Test default behavior
+    p3 = Product([1, 2, 3])
+    @test p3.metadata isa NoMetadata
+end

test/product.jl

@@ -0,0 +1,94 @@
+@testitem "Product Construction" begin
+    using SpaceDataModel: Product, func, NoMetadata
+
+    # Test basic construction with minimal parameters
+    p1 = Product([1, 2, 3])
+    @test parent(p1) == [1, 2, 3]
+    @test func(p1) === identity
+    @test p1.name == ""
+    @test p1.metadata isa NoMetadata  # NoMetadata() becomes Dict when merged with kwargs
+
+    # Test construction with transformation function
+    p2 = Product([1, 2, 3], x -> x .* 2)
+    @test parent(p2) == [1, 2, 3]
+    @test func(p2)([1, 2, 3]) == [2, 4, 6]
+
+    # Test construction with name
+    p3 = Product([1, 2, 3]; name="test_data")
+    @test p3.name == "test_data"
+
+    # Test construction with metadata
+    p4 = Product([1, 2, 3]; metadata=Dict("units" => "m/s"))
+    @test p4.metadata["units"] == "m/s"
+
+    # Test construction with kwargs that become metadata
+    p5 = Product([1, 2, 3]; units="m/s", description="velocity")
+    @test p5.metadata isa Base.Pairs
+    @test p5.metadata[:units] == "m/s"
+    @test p5.metadata[:description] == "velocity"
+
+    # Test full constructor
+    p6 = Product([1, 2, 3], x -> x .+ 1, "full_test", Dict("key" => "value"))
+    @test parent(p6) == [1, 2, 3]
+    @test func(p6)([1, 2, 3]) == [2, 3, 4]
+    @test p6.name == "full_test"
+    @test p6.metadata["key"] == "value"
+end
+
+@testitem "Product Methods" begin
+    using SpaceDataModel: Product
+    p1 = Product([1, 4, 9], x -> sqrt.(x))
+    @test all(p1 .== [1, 4, 9])
+    @test_nowarn sprint(show, p1)
+end
+
+@testitem "Product Function Application and Composition" begin
+    using SpaceDataModel: Product
+
+    p1 = Product([1, 2, 3], x -> x .* 2)
+    @test p1() == [2, 4, 6]
+
+    # Test calling with additional arguments
+    p2 = Product([1, 2, 3], (x, y) -> x .+ y)
+    @test p2(10) == [11, 12, 13]
+
+    # Test with identity transformation
+    p3 = Product([1, 2, 3])
+    @test p3() == [1, 2, 3]
+
+    # Test composition with function on left
+    p1 = Product([1, 2, 3], x -> x .* 2)
+    p2 = Product([1, 2, 3], x -> x .+ 1)
+    p3 = (x -> x .+ 1) ∘ p1
+    @test p3() == [3, 5, 7]  # (x * 2) + 1
+
+    # Test composition with function on right
+    p4 = p1 ∘ (x -> x .+ 1)
+    @test p4() == [4, 6, 8]  # (x + 1) * 2
+
+    # Test composition between products
+    p5 = p1 ∘ p2
+    @test p5() == [4, 6, 8]  # (x + 1) * 2
+end
+
+@testitem "Product Setting Operations" begin
+    using SpaceDataModel: Product, set
+
+    # Test immutable set operations
+    p1 = Product([1, 2, 3]; name="original")
+
+    # Test setting name
+    p2 = set(p1; name="new_name")
+    @test p2.name == "new_name"
+    @test p1.name == "original"  # Original unchanged
+
+    # Test setting data
+    data = [4, 5, 6]
+    @test set(p1; data)() == data
+    @test set(p1, data)() == data
+
+    # Test setting metadata
+    p4 = set(p1; units="m/s", description="velocity")
+    @test p4.metadata[:units] == "m/s"
+    @test p4.metadata[:description] == "velocity"
+end