Skip to content

Conversation

@Mukulyadav2004
Copy link
Contributor

Closes #6855

This PR replaces partial argument names (id, fun.agg, measure) with their full names (id.vars, fun.aggregate, measure.vars) in the following vignettes for improved clarity and consistency:

->vignettes/data.table-reshape.Rmd
->vignettes/fr/data.table-reshape.Rmd
->vignettes/ru/data.table-reshape.Rmd
Additionally, these changes ensure better alignment with the surrounding text for improved coherence.

@MichaelChirico, please review when you have time. Thank you!

@Mukulyadav2004 Mukulyadav2004 requested review from a team and tdhock as code owners March 16, 2025 03:00
@codecov
Copy link

codecov bot commented Mar 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.59%. Comparing base (55eb0f1) to head (440cca6).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6870   +/-   ##
=======================================
  Coverage   98.59%   98.59%           
=======================================
  Files          79       79           
  Lines       14660    14660           
=======================================
  Hits        14454    14454           
  Misses        206      206           

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

@aitap aitap left a comment

Choose a reason for hiding this comment

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

Tested by re-rendering with options(warnPartialMatchArgs=TRUE, warn=2).

@ben-schwen
Copy link
Member

Ideally, we remove partial arguments from all vignettes in one go, right?

@aitap
Copy link
Contributor

aitap commented Mar 16, 2025 via email

@ben-schwen
Copy link
Member

There could be more in non-executable chunks, but in my tests, only data.table-reshape.Rmd previously failed with options(warnPartialMatchArgs=TRUE, warn=2).

You are right, I just manually build all vignettes with tools::buildVignette() and only the reshape vignette failed

@Mukulyadav2004
Copy link
Contributor Author

I have removed partial matching in the reshaping vignette since it was causing build failures with options(warnPartialMatchArgs=TRUE, warn=2). The other instances in comments and tests are either intentional or not causing any issues.
I kept this PR focused on resolving that issue. However, I’m open to addressing partial matching in other non-executable chunks if needed.

@MichaelChirico
Copy link
Member

Thank you @Mukulyadav2004!!

@MichaelChirico MichaelChirico merged commit 23a2f2d into Rdatatable:master Mar 16, 2025
10 checks passed
@Mukulyadav2004 Mukulyadav2004 deleted the replace_partial_matches branch March 16, 2025 19:37
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.

Typo in vignette on reshaping

4 participants