Repository: JuliaML/TableTransforms.jl (julia)

Issue:
Revert of `DropConstant` assumes same number of rows
MWE:

```julia
julia> d = georef((a=1:3, b=fill(1,3)))
3×3 GeoTable over 3 CartesianGrid{1,Float64}
┌─────────────┬─────────────┬─────────────────────────┐
│      a      │      b      │        geometry         │
│ Categorical │ Categorical │         Segment         │
│  [NoUnits]  │  [NoUnits]  │                         │
├─────────────┼─────────────┼─────────────────────────┤
│      1      │      1      │ Segment((0.0,), (1.0,)) │
│      2      │      1      │ Segment((1.0,), (2.0,)) │
│      3      │      1      │ Segment((2.0,), (3.0,)) │
└─────────────┴─────────────┴─────────────────────────┘

julia> n, c = apply(DropConstant(), d)
(3×2 GeoTable over 3 CartesianGrid{1,Float64}, ((Select([:a], nothing), ([:a], [1], [:b], [[1, 1, 1]], Union{Nothing, Int64}[2])), nothing))

julia> nn = georef((a=1:6,))
6×2 GeoTable over 6 CartesianGrid{1,Float64}
┌─────────────┬─────────────────────────┐
│      a      │        geometry         │
│ Categorical │         Segment         │
│  [NoUnits]  │                         │
├─────────────┼─────────────────────────┤
│      1      │ Segment((0.0,), (1.0,)) │
│      2      │ Segment((1.0,), (2.0,)) │
│      3      │ Segment((2.0,), (3.0,)) │
│      4      │ Segment((3.0,), (4.0,)) │
│      5      │ Segment((4.0,), (5.0,)) │
│      6      │ Segment((5.0,), (6.0,)) │
└─────────────┴─────────────────────────┘

julia> revert(DropConstant(), nn, c)
Error showing value of type GeoTable{CartesianGrid{1, Float64}, NamedTuple{(:a, :b), Tuple{UnitRange{Int64}, Vector{Int64}}}}:
ERROR: BoundsError: attempt to access 3-element Vector{Int64} at index [4]
```

Pull request description:
closes #253

Patch:
diff --git a/src/transforms.jl b/src/transforms.jl
index df771d29..8ce9e6f1 100644
--- a/src/transforms.jl
+++ b/src/transforms.jl
@@ -277,6 +277,7 @@ include("transforms/filter.jl")
 include("transforms/dropmissing.jl")
 include("transforms/dropextrema.jl")
 include("transforms/dropunits.jl")
+include("transforms/dropconstant.jl")
 include("transforms/absoluteunits.jl")
 include("transforms/map.jl")
 include("transforms/replace.jl")
diff --git a/src/transforms/dropconstant.jl b/src/transforms/dropconstant.jl
new file mode 100644
index 00000000..8e2762b4
--- /dev/null
+++ b/src/transforms/dropconstant.jl
@@ -0,0 +1,50 @@
+# ------------------------------------------------------------------
+# Licensed under the MIT License. See LICENSE in the project root.
+# ------------------------------------------------------------------
+
+"""
+    DropConstant()
+
+Drops the constant columns using the `allequal` function.
+"""
+struct DropConstant <: StatelessFeatureTransform end
+
+isrevertible(::Type{DropConstant}) = true
+
+function applyfeat(::DropConstant, feat, prep)
+  cols = Tables.columns(feat)
+  names = Tables.columnnames(cols) |> collect
+
+  # constant columns
+  cnames = filter(names) do name
+    x = Tables.getcolumn(cols, name)
+    allequal(x)
+  end
+  cinds = indexin(cnames, names)
+  cvalues = [first(Tables.getcolumn(cols, nm)) for nm in cnames]
+
+  # selected columns
+  snames = setdiff(names, cnames)
+  columns = [Tables.getcolumn(cols, nm) for nm in snames]
+
+  𝒯 = (; zip(snames, columns)...)
+  newfeat = 𝒯 |> Tables.materializer(feat)
+  newfeat, (cinds, cnames, cvalues)
+end
+
+function revertfeat(::DropConstant, newfeat, fcache)
+  cols = Tables.columns(newfeat)
+  names = Tables.columnnames(cols) |> collect
+  columns = Any[Tables.getcolumn(cols, name) for name in names]
+
+  cinds, cnames, cvalues = fcache
+
+  nrows = _nrows(newfeat)
+  for (i, cind) in enumerate(cinds)
+    insert!(names, cind, cnames[i])
+    insert!(columns, cind, fill(cvalues[i], nrows))
+  end
+
+  𝒯 = (; zip(names, columns)...)
+  𝒯 |> Tables.materializer(newfeat)
+end
diff --git a/src/transforms/satisfies.jl b/src/transforms/satisfies.jl
index 27aeefac..ba7b8791 100644
--- a/src/transforms/satisfies.jl
+++ b/src/transforms/satisfies.jl
@@ -66,10 +66,3 @@ Except(DST.Categorical)
 ```
 """
 Except(S::Type{<:SciType}) = Satisfies(x -> !(elscitype(x) <: S))
-
-"""
-    DropConstant()
-
-Drops the constant columns using the `allequal` function.
-"""
-DropConstant() = Satisfies(!allequal)


Test patch:
diff --git a/test/shows.jl b/test/shows.jl
index a1d163b8..68938501 100644
--- a/test/shows.jl
+++ b/test/shows.jl
@@ -191,6 +191,32 @@
     └─ high = 0.75"""
   end
 
+  @testset "DropUnits" begin
+    T = DropUnits(:a, :b, :c)
+
+    # compact mode
+    iostr = sprint(show, T)
+    @test iostr == "DropUnits([:a, :b, :c])"
+
+    # full mode
+    iostr = sprint(show, MIME("text/plain"), T)
+    @test iostr == """
+    DropUnits transform
+    └─ selector = [:a, :b, :c]"""
+  end
+
+  @testset "DropConstant" begin
+    T = DropConstant()
+
+    # compact mode
+    iostr = sprint(show, T)
+    @test iostr == "DropConstant()"
+
+    # full mode
+    iostr = sprint(show, MIME("text/plain"), T)
+    @test iostr == "DropConstant transform"
+  end
+
   @testset "Map" begin
     fun = (a, b) -> 2a + b
     T = Map(:a => sin, [:a, :b] => fun => :c)
diff --git a/test/transforms.jl b/test/transforms.jl
index 3cf6d9ad..d536235f 100644
--- a/test/transforms.jl
+++ b/test/transforms.jl
@@ -11,6 +11,7 @@ transformfiles = [
   "dropmissing.jl",
   "dropextrema.jl",
   "dropunits.jl",
+  "dropconstant.jl",
   "absoluteunits.jl",
   "map.jl",
   "replace.jl",
diff --git a/test/transforms/dropconstant.jl b/test/transforms/dropconstant.jl
new file mode 100644
index 00000000..2c3b733d
--- /dev/null
+++ b/test/transforms/dropconstant.jl
@@ -0,0 +1,27 @@
+@testset "DropConstant" begin
+  @test isrevertible(DropUnits())
+
+  a = [4, 6, 7, 8, 1, 2]
+  b = fill(5, 6)
+  c = [1.9, 7.4, 8.6, 8.9, 2.4, 7.7]
+  d = fill(5.5, 6)
+  t = Table(; a, b, c, d)
+
+  T = DropConstant()
+  n, c = apply(T, t)
+  @test Tables.schema(n).names == (:a, :c)
+  @test Tables.getcolumn(n, :a) == t.a
+  @test Tables.getcolumn(n, :c) == t.c
+  tₒ = revert(T, n, c)
+  @test t == tₒ
+
+  # revert with different number of rows
+  T = DropConstant()
+  _, c = apply(T, t)
+  n = Table(; a=[t.a; [3, 4, 7, 2]], c=[t.c; [5.3, 4.9, 3.1, 6.8]])
+  r = revert(T, n, c)
+  @test r.a == n.a
+  @test r.b == fill(5, 10)
+  @test r.c == n.c
+  @test r.d == fill(5.5, 10)
+end
diff --git a/test/transforms/satisfies.jl b/test/transforms/satisfies.jl
index 4af20004..ef077634 100644
--- a/test/transforms/satisfies.jl
+++ b/test/transforms/satisfies.jl
@@ -71,19 +71,3 @@ end
   tₒ = revert(T, n, c)
   @test t == tₒ
 end
-
-@testset "DropConstant" begin
-  a = [4, 6, 7, 8, 1, 2]
-  b = fill(5, 6)
-  c = [1.9, 7.4, 8.6, 8.9, 2.4, 7.7]
-  d = fill(5.5, 6)
-  t = Table(; a, b, c, d)
-
-  T = DropConstant()
-  n, c = apply(T, t)
-  @test Tables.schema(n).names == (:a, :c)
-  @test Tables.getcolumn(n, :a) == t.a
-  @test Tables.getcolumn(n, :c) == t.c
-  tₒ = revert(T, n, c)
-  @test t == tₒ
-end