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

Address issue #396 #399

Merged
merged 3 commits into from
Aug 18, 2020
Merged

Address issue #396 #399

merged 3 commits into from
Aug 18, 2020

Conversation

JJSteph
Copy link
Contributor

@JJSteph JJSteph commented Aug 18, 2020

Description

Accounted for precision error in rounding.

Related Issue

Should fix issue #396.

Example

round_half_up(2436.845, 2)
[1] 2436.85

round_half_up(123.456,1)
[1] 123.5
round_half_up(123.456,2)
[1] 123.46
round_half_up(123.456,3)
[1] 123.456

round_half_up(-123.456,1)
[1] -123.5
round_half_up(-123.456,2)
[1] -123.46
round_half_up(-123.456,3)
[1] -123.456

@codecov
Copy link

codecov bot commented Aug 18, 2020

Codecov Report

Merging #399 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #399   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines          901       901           
=========================================
  Hits           901       901           
Impacted Files Coverage Δ
R/round_half_up.R 100.00% <100.00%> (ø)

@sfirke
Copy link
Owner

sfirke commented Aug 18, 2020

Thank you for jumping in! For this pull request to be completed, it needs:

  • An edit to the NEWS.md file adding a bullet at the top, recognizing the change and that you fixed it
  • A new test that confirms that round_half_up(2436.845, 2) yields 2436.85. Basically what you showed above in pasting your outputs, but it would become a permanent part of the test suite. You could add it after line 12 here, following that same format.

Then I can merge it. Let me know if you have any questions.

Adding test for issue #396.
Added bullet point for fix #396.
@sfirke
Copy link
Owner

sfirke commented Aug 18, 2020

Looks great, thank you for contributing! 🙌

@sfirke sfirke merged commit cbafdd1 into sfirke:master Aug 18, 2020
@JJSteph JJSteph deleted the iss396 branch August 18, 2020 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants