Skip to content
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.

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

reduce tolerence #83

Merged
merged 8 commits into from
Nov 18, 2024
Merged

reduce tolerence #83

merged 8 commits into from
Nov 18, 2024

Conversation

shajoezhu
Copy link
Collaborator

@shajoezhu shajoezhu commented Nov 18, 2024

Pull Request

Fixes #82

currently still failing for macos, see https://github.com/insightsengineering/tern.rbmi/actions/runs/11885263587 for more details

@shajoezhu shajoezhu added the sme label Nov 18, 2024
Copy link
Contributor

github-actions bot commented Nov 18, 2024

badge

Code Coverage Summary

Filename             Stmts    Miss  Cover    Missing
-----------------  -------  ------  -------  ---------
R/tabulate_rbmi.R       82       0  100.00%
TOTAL                   82       0  100.00%

Diff against main

Filename      Stmts    Miss  Cover
----------  -------  ------  --------
TOTAL             0       0  +100.00%

Results for commit: 769ab24

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Nov 18, 2024

Unit Tests Summary

7 tests   6 ✅  0s ⏱️
3 suites  1 💤
1 files    0 ❌

Results for commit 769ab24.

♻️ This comment has been updated with latest results.

@shajoezhu shajoezhu requested a review from pawelru November 18, 2024 03:58
@pawelru
Copy link
Contributor

pawelru commented Nov 18, 2024

This looks good - it would be great if we can test it. However it fails on some unrelated step before. I actually observed identical error in other repos as well so it might be some bug in the central testing infrastructure.

@pawelru
Copy link
Contributor

pawelru commented Nov 18, 2024

The underlaying issue had been fixed. I re-run the workflow and it continues to fail. Please have a look: https://github.com/insightsengineering/tern.rbmi/actions/runs/11885263587

@pawelru
Copy link
Contributor

pawelru commented Nov 18, 2024

Looking into the test itself. It looks to me that these are string (!) comparison - not numerical values. Hence the tolerance parameter does not change anything. Can we convert into numeric comparison with tolerance?

Copy link
Contributor

@pawelru pawelru left a comment

Choose a reason for hiding this comment

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

Saw the test results and it's passing. LGTM 👍

@pawelru pawelru self-assigned this Nov 18, 2024
@shajoezhu shajoezhu merged commit af26477 into main Nov 18, 2024
26 checks passed
@shajoezhu shajoezhu deleted the 82_accuracy branch November 18, 2024 21:49
@github-actions github-actions bot locked and limited conversation to collaborators Nov 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] R-4.4-mac
2 participants