Skip to content

add precision column stat_string and different table names #986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Melkiades merged 9 commits into from
Jan 27, 2025
Merged

add precision column stat_string and different table names #986

Melkiades merged 9 commits into from
Jan 27, 2025

Conversation

@Melkiades
Copy link
Contributor

@Melkiades Melkiades commented Jan 27, 2025 •
edited
Loading

Fix #977 #976 #970

@github-actions
Copy link
Contributor

github-actions bot commented Jan 27, 2025 •
edited
Loading

Unit Tests Summary

    1 files     28 suites   1m 35s ⏱️
  220 tests   220 ✅ 0 💤 0 ❌
1 586 runs  1 586 ✅ 0 💤 0 ❌

Results for commit a79baf6.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 27, 2025 •
edited
Loading

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
Result Data Frames 💔 $7.13$ $+2.31$ $+7$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
Result Data Frames 👶 $+0.72$ make_ard_works_if_string_precision_is_needed
Result Data Frames 👶 $+0.18$ make_ard_works_with_split_cols_by_multivar

Results for commit c83e988

♻️ This comment has been updated with latest results.

@Melkiades Melkiades linked an issue Jan 27, 2025 that may be closed by this pull request
Consider making the table names as an optional addition of make_ard = TRUE #976
Closed
@Melkiades Melkiades linked an issue Jan 27, 2025 that may be closed by this pull request
[Bug]: When using split_cols_by_multivar in layout, make_ard=T doesn't work #970
Closed
edelarua
edelarua approved these changes Jan 27, 2025
View reviewed changes
Copy link
Contributor

@edelarua edelarua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good!! Thanks Davide! Just one typo in the NEWS file to correct.

@Melkiades @edelarua
Update NEWS.md
a79baf6 
Co-authored-by: Emily de la Rua <59304861+edelarua@users.noreply.github.com>
Signed-off-by: Davide Garolini <dgarolini@gmail.com>
@Melkiades Melkiades enabled auto-merge (squash) January 27, 2025 17:01
@Melkiades Melkiades merged commit 088d4bb into main Jan 27, 2025
28 checks passed
@Melkiades Melkiades deleted the 977_add_precision@main branch January 27, 2025 17:11
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ayogasekaram ayogasekaram Awaiting requested review from ayogasekaram ayogasekaram is a code owner

1 more reviewer

@edelarua edelarua edelarua approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ARD sme

Projects

None yet

Milestone

No milestone

3 participants

@Melkiades @edelarua