alexsanjoseph / compareDF

R Tool to compare two data.frames
Other
93 stars 17 forks source link

Add headers parameter #15

Closed jdbarillas closed 5 years ago

jdbarillas commented 5 years ago

I added a parameter that takes a character vector to be used as column names for the htmlTable. I also added a check to make sure that the length of that vector is equal to that of the table.

An added side effect is that you can add html within the character vector which will be reflected in the htmlTable output.

old_df = data.frame(var1 = c("A", "B", "C"),
                    val1 = c(1, 2, 3))
new_df = data.frame(var1 = c("A", "B", "C"),
                    val1 = c(1, 2, 4))
ctable = compare_df(new_df, old_df, c("var1"), 
                    headers = c("<i>varA</i>", "<span style='color:red;'>valB</span>"))
print(ctable$comparison_df)
ctable$html_output

Please feel free to critique, I don't have much experience with contributing.

alexsanjoseph commented 5 years ago

Thanks for the PR, the commit looks good. However, the build seems to be failing? And I'd add a small test as well.

Currently I'm a bit strapped for time, so if you can get to fixing ^, please go ahead. Else I'll work on it within a week or so

Thanks again, Alex

jdbarillas commented 5 years ago

I turned the addition of the headers into the function and also created some tests for it. I believe the builds are failing due to packages. I wonder if it has something to do with me working from Linux.

Thank you again for the feedback, it's been extremely helpful.

alexsanjoseph commented 5 years ago

Excellent PR from someone who claims to have little experience with contributing. I've made a few changes in implementation here. Please take a look. If it's alright with you I shall merge.

@jdbarillas

https://github.com/alexsanjoseph/compareDF/pull/16

jdbarillas commented 5 years ago

Thank you for the compliment and feedback! I learned some new things going through your changes, and I'm alright with merging the PR.

alexsanjoseph commented 5 years ago

Closed along with PR/16