fixed AssertionError

[vickydeka]

@juliohm I fixed the Assertion Error and it is working fine now, then again was getting ERROR: MethodError: no method matching getindex(::NamedTuple{(:a, :b), Tuple{Vector{Int64}, Vector{Float64}}}, ::Colon, ::Vector{Symbol}) for

(a=rand(Int,3), b=rand(3)) |> ZScore(:b)

I guess the ZScore function expects the input data to be a tabular data structure, like DataFrame or a Table, but the input data in this case is a NamedTuple. Converting into dataframe, I am getting correct output:

julia> DataFrame((a=rand(Int,3), b=rand(3))) |> ZScore(:b)
3×2 DataFrame
 Row │ a                    b
     │ Int64                Float64
─────┼────────────────────────────────
   1 │ 4442619791281385888  -1.12529
   2 │ 7630774422502565167   0.786904
   3 │ 1978852580104395244   0.338386

[juliohm]

Still missing tests as suggested in the review.

[vickydeka]

Still missing tests as suggested in the review.

I am not exactly sure, how to add that.

[juliohm]

You can copy the example that was failing in our test/transforms/zscore.jl file and add check the expected result. You can find examples in the file. After you add these, GitHub will run them automatically to make sure that the tests are passing in all platforms.

[vickydeka]

and add check the expected res

a=rand(Int,3) 
b=rand(3)
t = Table(; a, b)
T = ZScore(:b)
n, c = apply(T, t)
@test isapprox(mean(n.b), 0; atol=1e-6)
@test isapprox(std(n.b), 1; atol=1e-6)
tₒ = revert(T, n, c)
@test Tables.matrix(t) ≈ Tables.matrix(tₒ)

Will in this way work?

[juliohm]

That should work 👍🏻

Em sáb., 10 de dez. de 2022 02:42, vickydeka @.***> escreveu:

and add check the expected res

a=rand(Int,3)

b=rand(3)

t = Table(; a, b)

T = ZScore(:b)

n, c = apply(T, t) @test isapprox(mean(n.b), 0; atol=1e-6) @test isapprox(std(n.b), 1; atol=1e-6)

tₒ = revert(T, n, c) @test Tables.matrix(t) ≈ Tables.matrix(tₒ)

Will in this way work?

— Reply to this email directly, view it on GitHub https://github.com/JuliaML/TableTransforms.jl/pull/160#issuecomment-1345147036, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3LCX62O72XX5DZ63QLWMQJ35ANCNFSM6AAAAAASZTR6HA . You are receiving this because you were mentioned.Message ID: @.***>

[vickydeka]

Please add the example that was failing to the test suite so that the issue is not reintroduced.

It is now added.

[codecov-commenter]

Codecov Report

Merging #160 (de468ec) into master (976ce71) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #160   +/-   ##
=======================================
  Coverage   95.15%   95.16%           
=======================================
  Files          27       27           
  Lines         888      889    +1     
=======================================
+ Hits          845      846    +1     
  Misses         43       43           

Impacted Files	Coverage Δ
src/transforms.jl	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[juliohm]

Thank you @vickydeka for the contribution 💯 The tests are failing for reasons that are not related to your changes.