Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Closes #2147 addressing cran failure #2149

Merged
merged 3 commits into from
Oct 4, 2023
Merged

Closes #2147 addressing cran failure #2149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
be6cb71
feat: #2147 one solution but proceed with caution
Oct 3, 2023
e12a166
description uplevel and news blurb
Oct 4, 2023
9e3f450
pass links?
Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Package: admiral
Type: Package
Title: ADaM in R Asset Library
Version: 0.12.1
Version: 0.12.2
Authors@R: c(
person("Ben", "Straub", email = "[email protected]", role = c("aut", "cre")),
person("Stefan", "Bundfuss", role = "aut"),
Expand Down
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# admiral 0.12.2

- A unit test for `derive_param_computed()` was modified in anticipation of major user-facing changes to R version 4.4 (#2147)

# admiral 0.12.1

- `derive_extreme_records()` no longer fails if `dataset_add` is specified and a
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Phases:
| Q4-2023 | December 4th | December 11th |
| | [{pharmaversesdtm}](https://pharmaverse.github.io/pharmaversesdtm/main/) | [{admiralonco}](https://pharmaverse.github.io/admiralonco/main/) |
| | [{admiraldev}](https://pharmaverse.github.io/admiraldev/main/) | [{admiralophtha}](https://pharmaverse.github.io/admiralophtha/main/) |
| | [{admiral}](https://pharmaverse.github.io/admiral/main/) | |
| | [{admiral}](https://pharmaverse.github.io/admiral) | |

The `{admiral}` Q4-2023 release will officially be `{admiral}`'s version 1.0.0 release, where we commit to increased package maturity and pivot towards focusing on maintenance rather than new content. This does not mean that there will never be any new content in `{admiral}`, rather it means we will be more mindful about introducing new functionality and/or breaking changes. The release schedule in 2024 and onward will also shift to twice-yearly, rather than quarterly, so that our users have ample time to react to any new content and changes that do make it onto `{admiral}`.

Expand Down
10 changes: 1 addition & 9 deletions tests/testthat/test-derive_param_computed.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,15 +343,7 @@ test_that("derive_param_computed Test 8: no new observations if a constant param
AVALU = "kg/m2"
)
),
regexp = paste(
paste(
"The input dataset does not contain any observations fullfiling the filter",
"condition (NULL) for the parameter codes (PARAMCD) `HEIGHT`"
Copy link
Collaborator Author

@zdz2101 zdz2101 Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

image

Background:

  • One of R version 4.4's biggest changes is that is.atomic(NULL) now returns FALSE, it currently returns TRUE.
  • In our codebase, for get_hori_data() which is used for derive_param_computed(), in the section that creates warning messaging, we use a function from rlang::expr_label() to assist us with the aesthetically pleasing looking warning messages.
  • expr_label() is currently under the lifecycle's "questioning" label which means rlang themselves had thoughts of: "a function is no longer certain that a function is the optimal approach, but doesn’t yet know how to do it better"
  • We would just need to add ticks around NULL to fix/make sure the test passes for 4.4 but it would break the tests for <=4.3
  • For the purpose of our code/test in this case its really no big deal, I suggest we just lighten up the restriction not to string match the entire warning message, and just the part that says: "The input dataset does not contain any observations fullfiling the filter" so that it would continue to work for all versions
  • The messaging of this warning in the future would now encase NULL in ticks vs prior to 4.4 it did not

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pharmaverse/admiral

),
"No new observations were added.",
sep = "\n"
),
fixed = TRUE
regexp = "The input dataset does not contain any observations fullfiling the filter"
)

expect_dfs_equal(
Expand Down
Loading