-
Notifications
You must be signed in to change notification settings - Fork 0
optionals with new_union default to NULL #2
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.
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
Changes from all commits
8f56e45
5ac5638
0a1ec0f
f83948c
64f692e
429ad9e
edbcdd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,10 @@ | ||
| ^__dev$ | ||
| ^\.github$ | ||
| ^.*\.code-workspace$ | ||
| ^revdep$ | ||
| ^specs$ | ||
| ^AGENTS\.md$ | ||
| ^CLAUDE\.md$ | ||
| ^Makefile$ | ||
| ^SKILL\.md$ | ||
| ^SKILL\.md$ | ||
| ^cran-comments\.md$ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| # R-hub's generic GitHub Actions workflow file. It's canonical location is at | ||
| # https://github.com/r-hub/actions/blob/v1/workflows/rhub.yaml | ||
| # You can update this file to a newer version using the rhub2 package: | ||
| # | ||
| # rhub::rhub_setup() | ||
| # | ||
| # It is unlikely that you need to modify this file manually. | ||
|
|
||
| name: R-hub | ||
| run-name: "${{ github.event.inputs.id }}: ${{ github.event.inputs.name || format('Manually run by {0}', github.triggering_actor) }}" | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| config: | ||
| description: 'A comma separated list of R-hub platforms to use.' | ||
| type: string | ||
| default: 'linux,windows,macos' | ||
| name: | ||
| description: 'Run name. You can leave this empty now.' | ||
| type: string | ||
| id: | ||
| description: 'Unique ID. You can leave this empty now.' | ||
| type: string | ||
|
|
||
| jobs: | ||
|
|
||
| setup: | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| containers: ${{ steps.rhub-setup.outputs.containers }} | ||
| platforms: ${{ steps.rhub-setup.outputs.platforms }} | ||
|
|
||
| steps: | ||
| # NO NEED TO CHECKOUT HERE | ||
| - uses: r-hub/actions/setup@v1 | ||
| with: | ||
| config: ${{ github.event.inputs.config }} | ||
| id: rhub-setup | ||
|
|
||
| linux-containers: | ||
| needs: setup | ||
| if: ${{ needs.setup.outputs.containers != '[]' }} | ||
| runs-on: ubuntu-latest | ||
| name: ${{ matrix.config.label }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| config: ${{ fromJson(needs.setup.outputs.containers) }} | ||
| container: | ||
| image: ${{ matrix.config.container }} | ||
|
|
||
| steps: | ||
| - uses: r-hub/actions/checkout@v1 | ||
| - uses: r-hub/actions/platform-info@v1 | ||
| with: | ||
| token: ${{ secrets.RHUB_TOKEN }} | ||
| job-config: ${{ matrix.config.job-config }} | ||
| - uses: r-hub/actions/setup-deps@v1 | ||
| with: | ||
| token: ${{ secrets.RHUB_TOKEN }} | ||
| job-config: ${{ matrix.config.job-config }} | ||
| - uses: r-hub/actions/run-check@v1 | ||
| with: | ||
| token: ${{ secrets.RHUB_TOKEN }} | ||
| job-config: ${{ matrix.config.job-config }} | ||
|
|
||
| other-platforms: | ||
| needs: setup | ||
| if: ${{ needs.setup.outputs.platforms != '[]' }} | ||
| runs-on: ${{ matrix.config.os }} | ||
| name: ${{ matrix.config.label }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| config: ${{ fromJson(needs.setup.outputs.platforms) }} | ||
|
|
||
| steps: | ||
| - uses: r-hub/actions/checkout@v1 | ||
| - uses: r-hub/actions/setup-r@v1 | ||
| with: | ||
| job-config: ${{ matrix.config.job-config }} | ||
| token: ${{ secrets.RHUB_TOKEN }} | ||
| - uses: r-hub/actions/platform-info@v1 | ||
| with: | ||
| token: ${{ secrets.RHUB_TOKEN }} | ||
| job-config: ${{ matrix.config.job-config }} | ||
| - uses: r-hub/actions/setup-deps@v1 | ||
| with: | ||
| job-config: ${{ matrix.config.job-config }} | ||
| token: ${{ secrets.RHUB_TOKEN }} | ||
| - uses: r-hub/actions/run-check@v1 | ||
| with: | ||
| job-config: ${{ matrix.config.job-config }} | ||
| token: ${{ secrets.RHUB_TOKEN }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,8 +50,7 @@ character_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_character_scalar <- new_property( | ||
| class = new_union(class_character, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_character), | ||
| validator = function(value) { | ||
| if ( | ||
| !is.null(value) && | ||
|
|
@@ -91,8 +90,7 @@ double_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_double_scalar <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if (!is.null(value) && (length(value) != 1L || is.na(value))) { | ||
| return("must be NULL or a double scalar") | ||
|
|
@@ -129,8 +127,7 @@ integer_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_integer_scalar <- new_property( | ||
| class = new_union(class_integer, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_integer), | ||
| validator = function(value) { | ||
| if (!is.null(value) && (length(value) != 1L || is.na(value))) { | ||
| return("must be NULL or an integer scalar (e.g. 1L)") | ||
|
|
@@ -166,8 +163,7 @@ pos_integer_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_pos_integer_scalar <- new_property( | ||
| class = new_union(class_integer, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_integer), | ||
| validator = function(value) { | ||
| if ( | ||
| !is.null(value) && (length(value) != 1L || is.na(value) || value <= 0L) | ||
|
|
@@ -206,8 +202,7 @@ logical_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_logical_scalar <- new_property( | ||
| class = new_union(class_logical, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_logical), | ||
| validator = function(value) { | ||
| if (!is.null(value) && (length(value) != 1L || is.na(value))) { | ||
| return("must be NULL or a logical scalar (TRUE or FALSE)") | ||
|
|
@@ -244,8 +239,7 @@ prob_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_prob_scalar <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if ( | ||
| !is.null(value) && | ||
|
|
@@ -284,8 +278,7 @@ unit_open_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_unit_open_scalar <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if ( | ||
| !is.null(value) && | ||
|
|
@@ -326,8 +319,7 @@ pos_double_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_pos_double_scalar <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if ( | ||
| !is.null(value) && | ||
|
|
@@ -366,8 +358,7 @@ nonneg_double_scalar <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_nonneg_double_scalar <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if ( | ||
| !is.null(value) && | ||
|
|
@@ -414,8 +405,7 @@ prob_vector <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_prob_vector <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if (is.null(value)) { | ||
| return(NULL) | ||
|
|
@@ -468,8 +458,7 @@ unit_open_vector <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_unit_open_vector <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if (is.null(value)) { | ||
| return(NULL) | ||
|
|
@@ -522,8 +511,7 @@ pos_double_vector <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_pos_double_vector <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if (is.null(value)) { | ||
| return(NULL) | ||
|
|
@@ -576,8 +564,7 @@ nonneg_double_vector <- new_property( | |
| #' @author EDG | ||
| #' @export | ||
| optional_nonneg_double_vector <- new_property( | ||
| class = new_union(class_double, NULL), | ||
| default = NULL, | ||
| class = new_union(NULL, class_double), | ||
| validator = function(value) { | ||
| if (is.null(value)) { | ||
| return(NULL) | ||
|
|
@@ -646,12 +633,12 @@ bounded_double_property <- function( | |
| function(v) v <= upper | ||
| } | ||
|
|
||
| cls <- if (nullable) new_union(class_double, NULL) else class_double | ||
| cls <- if (nullable) new_union(NULL, class_double) else class_double | ||
|
|
||
| new_property( | ||
| class = cls, | ||
| validator = function(value) { | ||
| if (is.null(value)) { | ||
| if (nullable && is.null(value)) { | ||
| return(NULL) | ||
| } | ||
|
Comment on lines
+641
to
643
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While updating the validator to check Currently, |
||
| if (length(value) != 1L || is.na(value) || !is.finite(value)) { | ||
|
|
@@ -682,11 +669,11 @@ bounded_double_property <- function( | |
| #' @examples | ||
| #' type_prop <- enum(c("string", "number", "boolean"), default = "string") | ||
| enum <- function(values, default = NULL, nullable = FALSE) { | ||
| cls <- if (nullable) new_union(class_character, NULL) else class_character | ||
| cls <- if (nullable) new_union(NULL, class_character) else class_character | ||
| new_property( | ||
| class = cls, | ||
| validator = function(value) { | ||
| if (is.null(value)) { | ||
| if (nullable && is.null(value)) { | ||
| return(NULL) | ||
| } | ||
|
Comment on lines
+676
to
678
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with the other changes in this PR, consider updating the class definition for |
||
| if (length(value) != 1L || is.na(value)) { | ||
|
|
@@ -727,5 +714,5 @@ optional <- function(type) { | |
| "{.var type} must be an S7 base class or S7 class." | ||
| ) | ||
| } | ||
| S7::new_union(type, NULL) | ||
| S7::new_union(NULL, type) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests don’t currently assert the new intended behavior that these optional_* properties can be omitted and will default to NULL (now relying on
new_union(NULL, ...)rather than an explicitdefault = NULL). Please add a test that constructs a class with one of these properties and verifies the field isNULLwhen not provided (and that construction succeeds).