Skip to content

Added format specifier for yearqtr#7713

Open
LunaticSage218 wants to merge 6 commits intoRdatatable:masterfrom
LunaticSage218:master
Open

Added format specifier for yearqtr#7713
LunaticSage218 wants to merge 6 commits intoRdatatable:masterfrom
LunaticSage218:master

Conversation

@LunaticSage218
Copy link
Copy Markdown

Thank you for contributing to data.table!

Please be sure to read our CONTRIBUTING guide. In particular, "Contributors are requested not to use code assistants if they are not able to evaluate license of the code provided by an assistant, and to provide proper citation."

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7713      +/-   ##
==========================================
- Coverage   99.04%   99.04%   -0.01%     
==========================================
  Files          87       87              
  Lines       17037    17128      +91     
==========================================
+ Hits        16874    16964      +90     
- Misses        163      164       +1     

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

Comment thread man/IDateTime.Rd Outdated
Co-authored-by: Benjamin Schwendinger <52290390+ben-schwen@users.noreply.github.com>
Comment thread R/IDateTime.R Outdated
yearqtr = function(x, format=c("numeric", "character")) {
format = match.arg(format)
if (format == "numeric") return(convertDate(as.IDate(x), "yearqtr"))
yr = convertDate(as.IDate(x), "year")
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.

we dont need 3 calls of as.IDate(x) and 2 calls of convertDate. Cache them if necessary and calulate the Quarter als round((yqtr - floor(yqtr)) * 4) + 1L

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

got it

Comment thread NEWS.md Outdated

9. `fread()` no longer replaces a literal header column name `"NA"` with an auto-generated `Vn` name when `na.strings` includes `"NA"`, [#5124](https://github.com/Rdatatable/data.table/issues/5124). Data rows still continue to parse `"NA"` as missing. Thanks @Mashin6 for the report and @shrektan for the fix.

10. `yearqtr()` now accepts an optional format specifier [#7694](https://github.com/Rdatatable/data.table/issues/7694). 'numeric' is the deafult, which preserves the original behavior, but 'character' formats the date like so: YYYYQ# (e.g. 2025Q2). Thanks to @jan-swissre for the report and @LunaticSage218 for the implementation.
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.

at least a typo in default.

Suggested change
10. `yearqtr()` now accepts an optional format specifier [#7694](https://github.com/Rdatatable/data.table/issues/7694). 'numeric' is the deafult, which preserves the original behavior, but 'character' formats the date like so: YYYYQ# (e.g. 2025Q2). Thanks to @jan-swissre for the report and @LunaticSage218 for the implementation.
10. `yearqtr()` gains an optional `format` argument [#7694](https://github.com/Rdatatable/data.table/issues/7694). 'numeric' is the deafult, which preserves the original behavior, but 'character' formats the date like so: YYYYQ# (e.g. 2025Q2). Thanks to @jan-swissre for the report and @LunaticSage218 for the implementation.

@ben-schwen
Copy link
Copy Markdown
Member

Solid first approach. Keep in mind that we strive for efficiency with data.table so before calculating smth twice in a short period, we most of the time cache it.

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