Repository: JuliaML/TableTransforms.jl
Pull request #15 (2021-11-14 17:34:15)

wip: implement Rename transform
This should be fully functional, but I think the usage of `push!!` will need to be redesigned to be more functional and efficient like other transforms. That is the place where I think I need a little help: I am not sure how to make it so that I don't need `push!!` and use `𝒯 = (; zip(anames, acols)...)` instead like the rest of the of the transforms.

Omar-Elrefaei: Ooof, my bad for half of these things. I guess that what you get from sending a PR of after-midnight code without a review. I will fix the formatting and naming right away, but that I am not 100% sure about is how to phase out `push!!` ?
Omar-Elrefaei: Is it as simple as looping through the results of `Tables.columnnames(table)`, replacing the names that need to be renamed, and then `(; zip(names, cols)...)` ??
juliohm: Exactly, you can write it in a single:

```julia
newcols = map(cols) do col
  # do the renaming
end
```

See the other transforms for examples.
Omar-Elrefaei: I'm sorry, I screwed up git a bit. I'll comment when it's ready for review.

Omar-Elrefaei: I believe its ready for another review. 
juliohm: @Omar-Elrefaei did you have time to update this PR?
Omar-Elrefaei: Unfortunately no, I haven't been getting a chance to continue the work.



On Thu, Nov 18, 2021, 9:06 AM Júlio Hoffimann ***@***.***>
wrote:

> @Omar-Elrefaei <https://github.com/Omar-Elrefaei> did you have time to
> update this PR?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/JuliaML/TableTransforms.jl/pull/15#issuecomment-972895773>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEIXXL6RMMX5QOWLZOVZCS3UMUB5NANCNFSM5IACTAYQ>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>

juliohm: @Omar-Elrefaei you mean that this PR should be closed?
Omar-Elrefaei: > @Omar-Elrefaei you mean that this PR should be closed?

No, if not today after my dayjob, I'll definitely be sending updates in the weekend.
codecov-commenter: # [Codecov](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML) Report
> Merging [#15](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML) (cb6e7fb) into [master](https://codecov.io/gh/JuliaML/TableTransforms.jl/commit/8547acf04466471f5377bd8d7aa64bc3cf174ac7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML) (8547acf) will **increase** coverage by `3.28%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15/graphs/tree.svg?width=650&height=150&src=pr&token=AvbWvvRKNM&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML)](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML)

```diff
@@            Coverage Diff             @@
##           master      #15      +/-   ##
==========================================
+ Coverage   88.16%   91.44%   +3.28%     
==========================================
  Files          13       14       +1     
  Lines         321      339      +18     
==========================================
+ Hits          283      310      +27     
+ Misses         38       29       -9     
```


| [Impacted Files](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML) | Coverage Δ | |
|---|---|---|
| [src/transforms.jl](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML#diff-c3JjL3RyYW5zZm9ybXMuamw=) | `97.82% <100.00%> (+2.17%)` | :arrow_up: |
| [src/transforms/rename.jl](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML#diff-c3JjL3RyYW5zZm9ybXMvcmVuYW1lLmps) | `100.00% <100.00%> (ø)` | |
| [src/transforms/sequential.jl](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML#diff-c3JjL3RyYW5zZm9ybXMvc2VxdWVudGlhbC5qbA==) | `92.59% <100.00%> (+29.62%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML). Last update [a6d67c9...cb6e7fb](https://codecov.io/gh/JuliaML/TableTransforms.jl/pull/15?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaML).

Omar-Elrefaei: I think it's ready for another review.
I have:
- [x] added the assert
- [x] added tests
- [x] refactored `_rename` out
- [x] made the varargs constructor
juliohm: Awesome improvements @Omar-Elrefaei , I am merging it.

The test that is failing is unrelated. For some reason one of our visual tests is failing randomly. Do you think you can take a look at this issue? Maybe we are missing some `Random.seed!` call somewhere?

Patch:
diff --git a/src/TableTransforms.jl b/src/TableTransforms.jl
index 060ff79..9e24008 100644
--- a/src/TableTransforms.jl
+++ b/src/TableTransforms.jl
@@ -31,6 +31,7 @@ export
   # built-in
   Select,
   Reject,
+  Rename,
   Identity,
   Center,
   Scale,
diff --git a/src/transforms.jl b/src/transforms.jl
index e035b13..82613ca 100644
--- a/src/transforms.jl
+++ b/src/transforms.jl
@@ -213,6 +213,7 @@ end
 
 include("transforms/identity.jl")
 include("transforms/select.jl")
+include("transforms/rename.jl")
 include("transforms/center.jl")
 include("transforms/scale.jl")
 include("transforms/zscore.jl")
diff --git a/src/transforms/rename.jl b/src/transforms/rename.jl
new file mode 100644
index 0000000..db80f8d
--- /dev/null
+++ b/src/transforms/rename.jl
@@ -0,0 +1,46 @@
+# ------------------------------------------------------------------
+# Licensed under the MIT License. See LICENSE in the project root.
+# ------------------------------------------------------------------
+
+"""
+    Rename(:col₁ => :newcol₁, :col₂ => :newcol₂, ..., :col₁ => :newcolₙ)
+
+The transform that renames `col₁` to `newcol₁`, `col₂` to `newcol₂`, ...
+"""
+struct Rename <: Stateless
+  names::Dict{Symbol,Symbol}
+end
+
+pairsyms(x::Pair) = Symbol(first(x)) => Symbol(last(x))
+
+Rename(names::Pair) = pairsyms(names) |> Dict |> Rename
+Rename(names...) = pairsyms.(names) |> Dict |> Rename
+
+function apply(transform::Rename, table)
+  _rename(transform.names, table)
+end
+
+function revert(transform::Rename, table, cache)
+  # reversing the key-value pairs of the Dict
+  newnames = Dict(new => old for (old, new) in transform.names)
+  _rename(newnames, table) |> first
+end
+
+
+function _rename(names, table)
+  oldnames = Tables.columnnames(table)
+
+  # check if requested renames exist in the table
+  @assert keys(names) ⊆ oldnames "invalid column names"
+
+  # use new names if necessary
+  newnames = map(oldnames) do oldname
+    oldname in keys(names) ? names[oldname] : oldname
+  end
+
+  cols = Tables.columns(table)
+  vals = [Tables.getcolumn(cols, name) for name in oldnames]
+  𝒯 = (; zip(newnames, vals)...) |> Tables.materializer(table)
+  
+  𝒯, nothing
+end
\ No newline at end of file


Test patch:
diff --git a/test/transforms.jl b/test/transforms.jl
index 16c86cf..6c5d57e 100644
--- a/test/transforms.jl
+++ b/test/transforms.jl
@@ -131,6 +131,59 @@
     @test n1 == n2
   end
 
+  @testset "Rename" begin
+    a = rand(4000)
+    b = rand(4000)
+    c = rand(4000)
+    d = rand(4000)
+    t = Table(; a, b, c, d)
+
+    T = Rename(Dict(:a => :x))
+    n, c = apply(T, t)
+    @test Tables.columnnames(n) == (:x, :b, :c, :d)
+    tₒ = revert(T, n, c)
+    @test t == tₒ
+
+    T = Rename(Dict(:a => :x, :c => :y))
+    n, c = apply(T, t)
+    @test Tables.columnnames(n) == (:x, :b, :y, :d)
+    tₒ = revert(T, n, c)
+    @test t == tₒ
+
+    # rename with string pairs
+    T = Rename("a" => "x", "c" => "y")
+    n, c = apply(T, t)
+    @test Tables.columnnames(n) == (:x, :b, :y, :d)
+    tₒ = revert(T, n, c)
+    @test t == tₒ
+
+    # rename with symbol pairs
+    T = Rename(:a => :x, :c => :y)
+    n, c = apply(T, t)
+    @test Tables.columnnames(n) == (:x, :b, :y, :d)
+    tₒ = revert(T, n, c)
+    @test t == tₒ
+
+    # rename with mixed pairs
+    T = Rename("a" => :x)
+    n, c = apply(T, t)
+    @test Tables.columnnames(n) == (:x, :b, :c, :d)
+    tₒ = revert(T, n, c)
+    @test t == tₒ
+    
+    T = Rename("a" => :x, :c => "y")
+    n, c = apply(T, t)
+    @test Tables.columnnames(n) == (:x, :b, :y, :d)
+    tₒ = revert(T, n, c)
+    @test t == tₒ
+
+    # reapply test
+    T = Rename(:b => :x, :d => :y)
+    n1, c1 = apply(T, t)
+    n2 = reapply(T, t, c1)
+    @test n1 == n2
+  end
+
   @testset "Identity" begin
     x = rand(4000)
     y = rand(4000)