From 6282d2610ae579f3aac87200a9089c0e99c661da Mon Sep 17 00:00:00 2001
From: Kevin Carlson <kevin@topos.institute>
Date: Sat, 29 Jun 2024 11:01:54 -0700
Subject: [PATCH] Fix construction of varacset from `FinDomFunctor` (#919)

* Correct constructor of acset from findomfunctor for varacset case

* Added test

* Fix parameter error in building acsets from certain findomfunctors

* Iteration of varset returns Attrvars

* ci being weird

* Remove spurious comment added to trigger CI

---------

Co-authored-by: Kevin Arlin <arlinkevind@gmail.com>
Co-authored-by: Evan Patterson <evan@epatters.org>
---
 src/categorical_algebra/CSets.jl   |  4 +++-
 src/categorical_algebra/FinSets.jl | 10 +++++++---
 test/categorical_algebra/CSets.jl  | 16 ++++++++++++----
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/categorical_algebra/CSets.jl b/src/categorical_algebra/CSets.jl
index a93c0abf5..6793fb89e 100644
--- a/src/categorical_algebra/CSets.jl
+++ b/src/categorical_algebra/CSets.jl
@@ -229,13 +229,15 @@ functor_key(expr::GATExpr{:generator}) = first(expr)
 function (::Type{ACS})(F::FinDomFunctor) where ACS <: ACSet
   X = if ACS isa UnionAll
     pres = presentation(dom(F))
-    ACS{(eltype(ob_map(F, c)) for c in generators(pres, :AttrType))...}()
+    ACS{(strip_attrvars(eltype(ob_map(F, c))) for c in generators(pres, :AttrType))...}()
   else
     ACS()
   end
   copy_parts!(X, F)
   return X
 end
+strip_attrvars(T) = T
+strip_attrvars(::Type{Union{AttrVar, T}}) where T = T
 
 """ Copy parts from a set-valued `FinDomFunctor` to an `ACSet`.
 """
diff --git a/src/categorical_algebra/FinSets.jl b/src/categorical_algebra/FinSets.jl
index d36eab24a..6b1b036f5 100644
--- a/src/categorical_algebra/FinSets.jl
+++ b/src/categorical_algebra/FinSets.jl
@@ -315,9 +315,13 @@ VarSet(i::Int) = VarSet{Union{}}(i)
 SetOb(s::VarSet{Union{}}) = FinSet(s)
 SetOb(s::VarSet{T}) where T = TypeSet{Union{AttrVar,T}}()
 FinSet(s::VarSet) = FinSet(s.n) #Note this throws away `T`, most accurate when thinking about tight `VarFunction`s.
-Base.iterate(set::VarSet{T}, args...) where T = iterate(1:set.n, args...)
+"""
+The iterable part of a varset is its collection of `AttrVar`s.
+"""
+Base.iterate(set::VarSet{T}, args...) where T = iterate(AttrVar.(1:set.n), args...)
 Base.length(set::VarSet{T}) where T = set.n
 Base.in(set::VarSet{T}, elem) where T = in(elem, 1:set.n)
+Base.eltype(set::VarSet{T}) where T = Union{AttrVar,T}
 
 
 abstract type AbsVarFunction{T} end # either VarFunction or LooseVarFunction
@@ -363,7 +367,7 @@ function is_monic(f::VarFunction)
   vals = [v.val for v in collect(f.fun)]
   return length(vals) == length(unique(vals))
 end
-is_epic(f::VarFunction) = AttrVar.(f.codom) ⊆ collect(f)
+is_epic(f::VarFunction) = AttrVar.(f.codom) ⊆ collect(f) #XXX: tested?
 
 compose(::IdentityFunction{TypeSet{T}}, f::AbsVarFunction{T}) where T = f
 compose(f::VarFunction{T}, ::IdentityFunction{TypeSet{T}}) where T = f
@@ -1288,7 +1292,7 @@ end
 
 # FIXME: Handle more specific diagrams? Now only VarSet colimits will be bipartite
 function universal(lim::BipartiteColimit{<:VarSet{T}}, cocone::Multicospan) where {T}
-  VarFunction{T}(map(AttrVar.(collect(apex(lim)))) do p 
+  VarFunction{T}(map(collect(apex(lim))) do p 
     for (l, csp) in zip(legs(lim), cocone)
       pre = preimage(l, p) # find some colimit leg which maps onto this part
       if !isempty(pre)
diff --git a/test/categorical_algebra/CSets.jl b/test/categorical_algebra/CSets.jl
index b93c04dff..d81523ced 100644
--- a/test/categorical_algebra/CSets.jl
+++ b/test/categorical_algebra/CSets.jl
@@ -559,9 +559,17 @@ w1,w2,w3 = ws = [WG{x}() for x in [Symbol,Bool,Int]]
 rem_part!(w1, :Weight, 1)
 @test [nparts(w, :Weight) for w in ws] == [1,2,2]
 
-A′ = WG{Bool}(FinDomFunctor(A))
-rem_part!(A,:Weight,2)
-@test is_isomorphic(A,A′)
+A′ = WG(FinDomFunctor(A))
+rem_part!(A,:Weight,2) #remove the dangling attrvar from A
+
+C = FinCat(SchWeightedGraph)
+obs = Dict(x=>x for x in ob_generators(C))
+homs = Dict(f=> f for f in hom_generators(C))
+F = FinDomFunctor(obs,homs,C,C)
+A′′ = WG(compose(F,FinDomFunctor(A)))
+#Test both A′ and A′′ for constructing an acset from 
+# an ACSetFunctor, respectively, a FinDomFunctorMap.
+@test A == A′ == A′′
 
 # Construct Tight/Loose ACSet Transformations
 #--------------------------------------------
@@ -826,4 +834,4 @@ hs = [:it,:ot]
 @test sum(h->is_cartesian(h,hs),homoms) == 12
 
 
-end
\ No newline at end of file
+end