Add StdNames

[ceferisbarov]

I am still trying to make the code more readable. Suggestions are welcome.

[juliohm]

Great start @ceferisbarov , left a first round of reviews.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.91%. Comparing base (25bb1d1) to head (50f3bf4).
Report is 378 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #68      +/-   ##
==========================================
+ Coverage   92.49%   92.91%   +0.42%     
==========================================
  Files          21       22       +1     
  Lines         506      536      +30     
==========================================
+ Hits          468      498      +30     
  Misses         38       38              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ceferisbarov]

@juliohm Test set grew too large. I think we can create a function to avoid repeating the same lines.

[juliohm]

That is looking great now @ceferisbarov. I have a final suggestion before we merge this. Can you please replace all tests you have written by simple tests of the form:

@test TableTransforms._camel("apple banana") == "AppleBanana"
@test TableTransforms._snake("apple banana") == "apple_banana"
...

That way we can make sure that we cover various types of strings.

After the above set of tests you can simply add two more tests with tables to make sure that apply and revert are working with column and row tables. Also reapply and isrevertible.

[juliohm]

I think we should organize these tests differently. For example, consider the camel case. Your would like to write a list of strings like ["apple banana", "Apple_banana", ...] and then apply the camel case function to all elements of the list to make sure all results are equal to "AppleBanana".

[ceferisbarov]

@juliohm I am trying something like this now, but I came across an edge case. What should TableTransforms._snake("_A") return? Do we strip spaces and underscores or not?

[ceferisbarov]

@juliohm I have added some tests with for loop. I am pretty sure I can replace them with @testset macro, documentation is not clear about syntax. I will add more tests after we agree on syntax and style.

[juliohm]

@ceferisbarov please request a review whenever you feel that the PR is ready. Attached you can find another round of suggestions.

[juliohm]

Can you also add tests for columns with special characters such as A-B, A#, etc?

[juliohm]

As I mentioned it should be easy to erase all special symbols as a pre processing step with a Regex for example. We just need to decide the valids delimiters. I don't think & is widely used in statistics as a delimiter. It is used as an interaction term. Em qua., 11 de mai. de 2022 05:46, Jafar Isbarov ***@***.***> escreveu:

…

***@***.**** commented on this pull request. ------------------------------ In test/transforms.jl <#68 (comment)> : > + name = "a&B" + @test TableTransforms._camel(name) == "A&B" + @test TableTransforms._snake(name) == "a&b" + @test TableTransforms._upper(name) == "A&B" + + name = "apple-tree" + @test TableTransforms._camel(name) == "Apple-tree" + @test TableTransforms._snake(name) == "apple-tree" + @test TableTransforms._upper(name) == "APPLE-TREE" + + name = "apple#" + @test TableTransforms._camel(name) == "Apple#" + @test TableTransforms._snake(name) == "apple#" + @test TableTransforms._upper(name) == "APPLE#" I would also suggest adding & to the list of delimiters. That being said, I don't think we can make this transform work well with complex column names, anyway. Maybe we can later add more specific transforms that deal with column names (such as Replace, but for column names). @juliohm <https://github.com/juliohm> @eliascarv <https://github.com/eliascarv> — Reply to this email directly, view it on GitHub <#68 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZQW3KPSFQ6FR5JIS57RFLVJNXXNANCNFSM5VFQKBEQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[juliohm]

I think we are almost done with this one @ceferisbarov , can you please take a look at this new set of revisions?

[juliohm]

@eliascarv anything else I missed before we can merge this?

[eliascarv]

For me everything is excellent.

[juliohm]

Another great addition @ceferisbarov ! 🎉