CRAN feedback for 0.5.0

[donyunardi]

We got CRAN feedback:

If there are references describing the methods in your package, please add these in the description field of your DESCRIPTION file in the form authors (year) authors (year) authors (year, ISBN:...) or if those are not available: with no space after 'doi:', 'arXiv:', 'https:' and angle brackets for auto-linking. (If you want to add a title as well please put it in quotes: "Title")

Some code lines in examples are commented out. Please never do that. Ideally find toy examples that can be regularly executed and checked. Lengthy examples (> 5 sec), can be wrapped in \donttest{}. Examples for unexported function calls_combine_by() in: calls_combine_by.Rd updateCountBar() in: countBars.Rd DFFilterStates() in: fetch_bs_color.Rd FilterState() in: ChoicesFilterState.Rd countBars.Rd DateFilterState.Rd DatetimeFilterState.Rd DFFilterStates.Rd FilterStateExpr.Rd LogicalFilterState.Rd RangeFilterState.Rd FilterStateExpr() in: ChoicesFilterState.Rd countBars.Rd DateFilterState.Rd DatetimeFilterState.Rd DFFilterStates.Rd FilterStateExpr.Rd LogicalFilterState.Rd RangeFilterState.Rd init_filtered_data() in: make_c_call.Rd RangeFilterState() in: toggle_button.Rd SEFilterStates() in: topological_sort.Rd teal.slice-package() in: variable_types.Rd

Please fix and resubmit.

Please make sure to merge the PR to release-candidate-v0.5.0 branch.

[vedhav]

Some code lines in examples are commented out. Please never do that.

I had a look to find commented code in examples, But I could not. Do we have them?

[vedhav]

Lengthy examples (> 5 sec), can be wrapped in \donttest{}.

I just ran the tests and examples. I don't think we have examples/tests that run over 5 seconds.

[vedhav]

Examples for unexported function

We seem to have two things we can try here as we discussed before:

1. Message the reviewer asking for a reconsideration and point out that we do this in teal.code
2. Remove them in one commit and revert them after the CRAN release, Hopefully after the CRAN submission this won't be a big issue as CRAN would already have teal.slice so getting from namespace would make sense.

[chlebowa]

Some code lines in examples are commented out. Please never do that.

I had a look to find commented code in examples, But I could not. Do we have them?

Could it be these? FilteredData.R

#' ### set_filter_state

FilteredDatasetDataframe.R

#' ## set_filter_state

---

Lengthy examples (> 5 sec), can be wrapped in \donttest{}.

I just ran the tests and examples. I don't think we have examples/tests that run over 5 seconds.

I sometimes get messages about examples timing out when running checks but this seems random to me. I suppose it depends on the machine, too. I wonder if it's the apps causing it somehow, even though we use if (interactive()) everywhere. They do run when using run_examples. Maybe we should add \donttest around apps, just in case?

Or maybe we can time all examples and see if there is anything that runs significantly longer than the rest?

---

Examples for unexported function

We seem to have two things we can try here as we discussed before:

1. Message the reviewer asking for a reconsideration and point out that we do this in teal.code
2. Remove them in one commit and revert them after the CRAN release, Hopefully after the CRAN submission this won't be a big issue as CRAN would already have teal.slice so getting from namespace would make sense.

It would certainly be faster to remove them and deal with questions or changes later. I think we should go with (2). There is a PR ready.

Vote :eyes: for writing the reviewer and :rocket: for removing examples.

[m7pr]

I reckon the only thing we are supposed to do here is the removal of examples for unexported functions. I would resubmit without those examples. PR is ready - we can merge during the daily. Later let's bring them back. A separate conversation with a submitter that runs independently is fine.

[m7pr]

For teal.code we only to this once in one example, maybe what's they did not complain that much as here https://github.com/insightsengineering/teal.code/blob/05dee4111881c8650a2218e38ca87478015cfed8/R/utils.R#L35

[averissimo]

Could it be these? FilteredData.R

#' ### set_filter_state

FilteredDatasetDataframe.R

#' ## set_filter_state

It must be, other text comments on the examples are widely used.

The only other one that could qualify is #' # fun = dplyr::filter but that's on the @description

[chlebowa]

I think code is allowed in Description though it's usually not a separate line.

[averissimo]

I just ran the tests and examples. I don't think we have examples/tests that run over 5 seconds.

The first requireNamespce("MultiAssayExperiment") may take more than 5s (sometimes 6s on my laptop in a fresh session)

It's not in the examples, but in the @examplesIf

tictoc::tic()
#devtools::run_examples(fresh = T)
requireNamespace("MultiAssayExperiment")
#> Loading required namespace: MultiAssayExperiment
tictoc::toc()
#> 5.261 sec elapsed

^{Created on 2024-02-05 with reprex v2.1.0}

I think code is allowed in Description though it's usually not a separate line.

I agree. Removing the extra # will probably solve it whatever the reviewer took notice. Description should be completely fine

[vedhav]

Weird that I get this below 5 (always below 4). But, this is the slowest part of running my tests as well.

library(teal.slice)
tictoc::tic()
requireNamespace("MultiAssayExperiment")
#> Loading required namespace: MultiAssayExperiment
tictoc::toc()
#> 3.246 sec elapsed

^{Created on 2024-02-05 with reprex v2.1.0}

[chlebowa]

Yeah, that would do it...

[chlebowa]

I agree. Removing the extra # will probably solve it whatever the reviewer took notice. Description should be completely fine

Just removing the # will make set_filter_state appear as code, which would just print the function body. Let's change it to free text: set filter state.

[chlebowa]

At least some of the examples with MAE use non-exported functions so they will go anyway. Let's check Marcin's branch for example timing.

[averissimo]

I've submitted {teal.slice} to R Hub with #548 merged in the release candidate for 0.5.0 (to see if we can get around the long example)

Build link here for tracking purposes

Earlier today I submitted to R Hub to see if the examples NOTE appeared and it did :cry:

(disregard the warning cleverly hidden from the screenshot as I submitted with an uncommitted test that was failing and has since been deleted from my laptop)

https://builder.r-hub.io/status/teal.slice_0.4.0.9039.tar.gz-d70dbcf5fc9b4ff489fc9ab8aa83fc01#L7240

[averissimo]

I've used \donttest tag around the examples that took it longer, due to the suggestion from the reviewer and some information I found online.

(from reviewer) Lengthy examples (> 5 sec), can be wrapped in \donttest{}.

However, on r-pkgs.org there is a footnote that says:

You can wrap the code in \dontrun{}*, so it is never run by example(). The example above would look like this if you used \dontrun{} instead of try().

* You used to be able to use \donttest{} for a similar purpose, but we no longer recommend it because CRAN sets a special flag that causes the code to be executed anyway.↩︎

If this is the case we might have to remove examples that use MAE::miniACC and functions.

Both data("miniACC", package = "MultiAssayExperiment") and requireNamespace("MultiAssayExperiment") take a long time to run.

[donyunardi]

Completed in db4e78993466320b1c1b96f687e08a541add15dd and e8dabf7b654e30b64bfdc805722cf0a6405064b8, and I resubmit the package.

We'll open a separate issue for more feedback from CRAN.

[m7pr]

Regarding the time of execution of examples on CRAN raised in NOTES. This is just NOTES, this should not be a deal-breaker. Warnings and Errors are deal-breakers.