Skip to content

Closes #3148 - 1.6 deprecations/deletions [create website] - #3166

Open
bms63 with Copilot wants to merge 8 commits into
mainfrom
copilot/fix-1-6-deprecations
Open

Closes #3148 - 1.6 deprecations/deletions [create website]#3166
bms63 with Copilot wants to merge 8 commits into
mainfrom
copilot/fix-1-6-deprecations

Conversation

Copilot AI commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Advances all functions in the admiral deprecation cycle one phase forward for v1.6.0, per the deprecation strategy.

Changes by Phase

Phase 4 — Removed

  • call_user_fun(): deleted R source, tests, snapshot, and NAMESPACE export

Phase 3 — Now errors (deprecate_stop)

Previously Phase 2 (warning since 1.4.0 / deprecated since 1.2.0):

  • derive_param_extreme_record()derive_extreme_event()
  • derive_var_dthcaus() / dthcaus_source()derive_vars_extreme_event() / event()
  • derive_var_extreme_dt() / derive_var_extreme_dtm() / date_source()derive_vars_extreme_event() / event()
  • get_summary_records()derive_summary_records()

For each: changed to deprecate_stop(when = "1.6.0"), removed @examples from roxygen/Rd, replaced all tests with a single expect_error(..., class = "lifecycle_error_deprecated"), deleted snapshot files.

Phase 2 — Now warnings (deprecate_warn)

Previously Phase 1 (message since 1.4.0):

  • derive_var_merged_summary()derive_vars_merged_summary()
  • derive_var_atoxgr_dir(abnormal_indicator = )low_indicator / high_indicator

For each: changed to deprecate_warn(when = "1.6.0") with updated details message ("will turn into an error at the beginning of 2028"), updated snapshot format from Message to Warning.

Documentation

  • Removed [get_summary_records()] from @seealso in derive_merged.R
  • Updated Lab Grading vignette: abnormal_indicatorhigh_indicator
  • NEWS.md updated with all phase transitions

Checklist

  • Place Closes #<insert_issue_number> into the beginning of your Pull Request Title (Use Edit button in top-right if you need to update)
  • Code is formatted according to the tidyverse style guide. Run styler::style_file() to style R and Rmd files
  • Updated relevant unit tests or have written new unit tests, which should consider realistic data scenarios and edge cases, e.g. empty datasets, errors, boundary cases etc. - See Unit Test Guide
  • If you removed/replaced any function and/or function parameters, did you fully follow the deprecation guidance?
  • Review the Cheat Sheet. Make any required updates to it by editing the file inst/cheatsheet/admiral_cheatsheet.pptx and re-upload a PDF and a PNG version of it to the same folder. (The PNG version can be created by taking a screenshot of the PDF version.)
  • Update to all relevant roxygen headers and examples, including keywords and families. Refer to the categorization of functions to tag appropriate keyword/family.
  • Run devtools::document() so all .Rd files in the man folder and the NAMESPACE file in the project root are updated appropriately
  • Address any updates needed for vignettes and/or templates
  • Update NEWS.md under the header # admiral (development version) if the changes pertain to a user-facing function (i.e. it has an @export tag) or documentation aimed at users (rather than developers). A Developer Notes section is available in NEWS.md for tracking developer-facing issues.
  • Build admiral site pkgdown::build_site() and check that all affected examples are displayed correctly and that all new functions occur on the "Reference" page.
  • Address or fix all lintr warnings and errors - lintr::lint_package()
  • Run R CMD check locally and address all errors and warnings - devtools::check()
  • Link the issue in the Development Section on the right hand side.
  • Address all merge conflicts and resolve appropriately
  • Pat yourself on the back for a job well done! Much love to your accomplishment!

Copilot AI linked an issue Jul 26, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix 1.6 deprecations and update functions Closes #3148 - 1.6 deprecations/deletions Jul 26, 2026
Copilot finished work on behalf of bms63 July 26, 2026 03:44
Copilot AI requested a review from bms63 July 26, 2026 03:44
@bms63

bms63 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

@copilot can you not change the when = "1.4", leave those as is as that informs users when the process started for the deprecation. so please revert that lines to the previous numbers. can you also make an update the claude.md or make a suggeation in the PR comment on what to change in the prog strategy so you have this for next time

Copilot AI commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot can you not change the when = "1.4", leave those as is as that informs users when the process started for the deprecation. so please revert that lines to the previous numbers. can you als...

Done in 60ad515. Reverted all when = values back to their original Phase 1 start versions:

  • "1.2.0"derive_param_extreme_record, get_summary_records, derive_var_dthcaus, derive_var_extreme_date
  • "1.4"derive_var_merged_summary (in derive_merged.R)
  • "1.4.0"derive_var_atoxgr_dir(abnormal_indicator=)

Also updated AGENTS.md with an explicit note under the Phase 1 deprecation template:

Important: The when argument must be set to the version in which the function or argument first entered the deprecation cycle (Phase 1) and must never be changed in subsequent phases. It is the historical record of when the deprecation process started, not the version of the current phase.

Copilot finished work on behalf of bms63 July 27, 2026 04:04
@bms63 bms63 changed the title Closes #3148 - 1.6 deprecations/deletions Closes #3148 - 1.6 deprecations/deletions [create website] Jul 28, 2026
@bms63
bms63 marked this pull request as ready for review July 28, 2026 00:57
@bms63

bms63 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

@copilot can you do an empty commit so it triggers the website build please

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@copilot can you do an empty commit so it triggers the website build please

Done — empty commit pushed (bc72467) to trigger the website build.

Copilot finished work on behalf of bms63 July 28, 2026 00:59
@github-actions

Copy link
Copy Markdown
Contributor

The website/documentation is available at https://pharmaverse.github.io/admiral/copilot/fix-1-6-deprecations.

@bms63

bms63 commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

@copilot can you investigate teh failing CMD checks and provide a potential solution. do not implement yet just provide the solution

Copilot AI commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot can you investigate teh failing CMD checks and provide a potential solution. do not implement yet just provide the solution

There are two distinct failure types:


Failure 1 — Snapshot mismatches (2 tests)

The lifecycle when = argument controls the version that appears in the warning/error message (e.g., "was deprecated in admiral X.Y.Z").

  • tests/testthat/_snaps/derive_var_atoxgr.md: The stored snapshot says "was deprecated in admiral 1.6.0" (written when when = "1.6.0" was first committed), but after reverting when to its original value "1.4.0", the actual warning now says "was deprecated in admiral 1.4.0" → snapshot mismatch.

  • tests/testthat/_snaps/derive_merged.md: Test 33 ("deprecation warning" for derive_var_merged_summary) uses expect_snapshot() but no snapshot entry for it exists in the file (it ends at Test 32). A missing snapshot is treated as a new/unaccepted snapshot → second snapshot failure.

Fix: Update both snapshot files to reflect the actual when = values:

  • derive_var_atoxgr.md: change "was deprecated in admiral 1.6.0""was deprecated in admiral 1.4.0"
  • derive_merged.md: add a snapshot entry for Test 33 with the warning text from deprecate_warn(when = "1.4", ...), i.e., containing "was deprecated in admiral 1.4"

Failure 2 — Roxygen / Man Pages out of date

Eight man/*.Rd files still reference call_user_fun() in their Other deprecated: cross-reference sections, but that function was deleted in this PR. The Roxygen CI job runs roxygen2::roxygenize('.') and detects the diff.

Fix: Remove the \code{\link[=call_user_fun]{call_user_fun()}}, line from each of these 8 files:

  • man/dthcaus_source.Rd
  • man/derive_var_dthcaus.Rd
  • man/date_source.Rd
  • man/derive_var_extreme_dtm.Rd
  • man/get_summary_records.Rd
  • man/derive_var_merged_summary.Rd
  • man/derive_var_extreme_dt.Rd
  • man/derive_param_extreme_record.Rd

Just let me know when you'd like me to go ahead and implement these fixes.

@bms63

bms63 commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

@copilot can you implemented these changes please

Create a Phase 1 (message) stub - will be blank for now but shoudl be similar layout to previous admiral version
Can we make this linkable by removing the arugment derive_var_atoxgr_dir(abnormal_indicator = ) from the function but keep the text for what has been changed.

Co-authored-by: bms63 <10111024+bms63@users.noreply.github.com>

Copilot AI commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot can you implemented these changes please

Create a Phase 1 (message) stub - will be blank for now but shoudl be similar layout to previous admiral version
...

Done in the latest commit. Here's a summary of what was implemented:

  1. Phase 1 stub added to NEWS.md — mirrors the layout from admiral 1.5.0 with "No functions or arguments in this phase."

  2. derive_var_atoxgr_dir() made linkable — changed what = "derive_var_atoxgr_dir(abnormal_indicator = )" to what = "derive_var_atoxgr_dir()" so the function name is hyperlinked in pkgdown. The text explaining the abnormal_indicatorlow_indicator/high_indicator change is preserved in the details bullets. Snapshot updated to match the new message format and correct when = "1.4.0".

  3. CI snapshot failures fixed:

    • derive_var_atoxgr.md: updated to reflect 1.4.0 and the new message format
    • derive_merged.R Test 33: changed from expect_snapshot() (missing snapshot) to expect_warning(class = "lifecycle_warning_deprecated") — simpler and avoids snapshot drift
  4. Man page call_user_fun() references removed from all 8 affected .Rd files (dthcaus_source, derive_var_dthcaus, date_source, derive_var_extreme_dtm, get_summary_records, derive_var_merged_summary, derive_var_extreme_dt, derive_param_extreme_record).

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Code Coverage

Package Line Rate Health
admiral 96%
Summary 96% (5929 / 6187)

@bms63

bms63 commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

@bundfussr and @manciniedoardo think this is ready for a pass through/review? Will fix the lint next weekend!!

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.

1.6 deprecations/deletions

2 participants