Skip to content

Improved verbose messages in rolling functions frolladaptivefun and frollfun#7733

Open
Omartech312 wants to merge 16 commits intoRdatatable:masterfrom
kkarissa:master
Open

Improved verbose messages in rolling functions frolladaptivefun and frollfun#7733
Omartech312 wants to merge 16 commits intoRdatatable:masterfrom
kkarissa:master

Conversation

@Omartech312
Copy link
Copy Markdown

Closes #7021.

Previously, the frolladaptivefun and rfollfun algo verbose messages were not super user friendly, displaying messages like:

frolladaptivefun: algo 0 not implemented, fall back to 1

The wanted adjustment was for it to instead produce:

frolladaptivefun: algo fast not implemented, fall back to exact

There was also:
frollfun: processing fun 0 algo 0 took...

and the desired result was instead:
frollfun: processing fun mean algo fast took...

We adjusted for the desired functionality by creating an array of strings called rfunNames, using the rfun enum as an index to select the proper display type for the running function and then displaying fast or exact dependent on the value of value (0 for fast, 1 for exact).

Here is an example of the current output (from running tests) with the changed files in this PR:
frolladaptivefun: processing fun MIN algo exact took 0.000s
frolladaptivefun: algo fast not implemented, fall back to exact

We have additionally adjusted the NEWS.md file and the corresponding tests in inst/tests/froll.Rraw.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.04%. Comparing base (da0d15e) to head (a51eb5d).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7733   +/-   ##
=======================================
  Coverage   99.04%   99.04%           
=======================================
  Files          87       87           
  Lines       17058    17064    +6     
=======================================
+ Hits        16895    16901    +6     
  Misses        163      163           

☔ 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.

Copy link
Copy Markdown
Member

@jangorecki jangorecki left a comment

Choose a reason for hiding this comment

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

Approach is good, just some comments to make it cleaner.

Comment thread inst/tests/froll.Rraw Outdated
Comment thread inst/tests/froll.Rraw Outdated
Comment thread src/froll.c Outdated
Comment thread src/frolladaptive.c Outdated
Comment thread src/froll.c Outdated
Comment thread src/frolladaptive.c
Comment thread src/frolladaptive.c
Comment thread src/frolladaptive.c Outdated
Comment thread src/frolladaptive.c Outdated
Comment thread NEWS.md Outdated
Comment thread inst/tests/froll.Rraw Outdated
Comment thread NEWS.md Outdated
@Omartech312 Omartech312 requested a review from jangorecki May 5, 2026 07:05
Comment thread inst/tests/froll.Rraw
"frollfun: processing fun MEAN algo fast took.*",
"frollfunR: processing.*took.*"
))
))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please revert change in this line

Copy link
Copy Markdown
Member

@jangorecki jangorecki left a comment

Choose a reason for hiding this comment

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

Trailing white spaces seems to be only remaining thing left.

Comment thread NEWS.md

6. `yearqtr()` and `yearmon()` now gain an optional format specifier [#7694](https://github.com/Rdatatable/data.table/issues/7694). 'numeric' is the default, which preserves the original behavior, but 'character' formats `yearqtr()` as YYYYQ# (e.g. 2025Q2) and `yearmon()` as YYYYM## (e.g. 2025M02, 2025M10). Thanks to @jan-swissre for the report and @LunaticSage218 for the implementation.

7. `frolladaptivefun()` and `frollfun()` adjusted print statements for verbose messages to be more user friendly, [#7021](https://github.com/Rdatatable/data.table/issues/7021) Thanks @jangorecki for initiating the issue.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

also needs your name or github tag for thanking yourself.

For the news phrasing itself, maybe something like?

Verbose outputs from `frolladaptivefun()` and `frollfun()` are now clearer and more user friendly

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.

Improve verbose messages in rolling functions

5 participants