setNumericRounding() returns old rounding value, instead of "NULL"

[joshhwuu]

Closes #6108

Changed setNumericRounding() to invisibly return the old value, allowing for this assignment:

old = setNumericRounding(2)     # store old value and initiate new value
setNumericRounding(old)         # restore old value

Should I include a change to test.data.table as outlined in the issue as well? Also, there are many instances where we use getNumericRounding() to set/restore numeric rounding and can be refactored following this change, are they worth changing?

[github-actions]

Generated via commit bb2f838

Download link for the artifact containing the test results: ↓ atime-results.zip

Time taken to finish the standard R installation steps: 11 minutes and 45 seconds

Time taken to run atime::atime_pkg on the tests: 3 minutes and 43 seconds

[MichaelChirico]

there are many instances where we use getNumericRounding() to set/restore numeric rounding and can be refactored following this change, are they worth changing?

Sorry, I'm not seeing those, unless you're referring to usage outside data.table itself?

[MichaelChirico]

Should I include a change to test.data.table as outlined in the issue as well?

Yes, the total change size is still pretty small --> safe to include here too. Thanks!

[joshhwuu]

Sorry, I'm not seeing those, unless you're referring to usage outside data.table itself?

Yeah, I meant in tests.Rraw, a bunch of tests do this

[MichaelChirico]

Yeah, I meant in tests.Rraw, a bunch of tests do this

Oh, I see it now. I guess tests.Rraw is not indexed for regex search by GitHub.

Let's mark it for follow-up since there are so many. Thanks!

[MichaelChirico]

Implementation looks good, only nit-picky details left to fix, thanks!

[joshhwuu]

Should we include a NEWS entry as well? Or is it minimal enough where it's not required?

[MichaelChirico]

Should we include a NEWS entry as well? Or is it minimal enough where it's not required?

I think it warrants a NOTE

[MichaelChirico]

LGTM!