klmr/box support #526

radbasa · 2023-03-06T07:25:14Z

Provide test coverage reports for klmr/box modules.

Closes #491

Coverage reports for box modules work best with covr::file_coverage().

What works:

Box module imports

box::use(
  prefix/module,
  alias = prefix/mod,
  prefix/module[attach_list],
  prefix/module[...],
  prefix/module[alias = attached_function]
)

R6 classes as box modules.

Changes:

Modified R/R6.R: replacements_R6 for code reuse in R/box.R R6 class support.
Unit tests and test applications for for the various ways of importing box modules.

Not implemented:

S4 and RC. klmr/box does not support S4.

Unfortunately S4 does not, and (I believe) fundamentally cannot ever work with ‘box’. The reason for that is that S4 explicitly only supports defining classes and methods in packages or the R global environment, and nothing else.
klmr/box#284 (comment)

See klmr/box issues:

Co-authored-by: Jakub Nowicki <q.nowicki@gmail.com>

Basic klmr/box support

Add covr visibility into private box module functions

Feature/r6 objects

…anup

Tests for box module attached functions, three dots, aliases

Feature/code cleanup

Pre pr cleanup

CLAassistant · 2023-03-06T07:25:20Z

All committers have signed the CLA.

radbasa · 2023-03-15T08:35:10Z

Hi @jimhester, several of the CICD steps fail with Error: Failed to get R 4.2.3: Failed to get R 4.2.3: Failed to download version 4.2.3: Error: Unexpected HTTP response: 403. Is there anything we can do on our end to resolve these?

jimhester · 2023-04-20T13:34:56Z

Could you look into the test errors on R-devel? https://github.com/r-lib/rig might be useful to get and use R-devel locally if you don't have it already installed.

jakubnowicki · 2023-04-21T08:02:22Z

Could you look into the test errors on R-devel? https://github.com/r-lib/rig might be useful to get and use R-devel locally if you don't have it already installed.

@jimhester we have tried that (up to setting a VM with Ubuntu 20.04 to be closer to the CI environment), but it works there. We will investigate.

* Added `replacements_box()` for `klmr/box` support. * Extracted `traverse_R6()` from `replacements_R6()` to reuse code in `replacements_box()`. * R6 class box modules test cases separated to handle a known R6 issues with `r-devel`. `skip_if(is_r_devel())` is used in the R6 test cases.

radbasa · 2023-10-10T05:05:56Z

Could you look into the test errors on R-devel? https://github.com/r-lib/rig might be useful to get and use R-devel locally if you don't have it already installed.

Hello @jimhester , the test errors on R-devel were caused by R6 classes in the test cases similar to the errors in test-R6.R addressed in commit 2b9b4dd. Like you, we could not replicate the error interactively. We separated the R6 test cases into separate tests and used skip_if(is_r_devel()) for the tests with R6 classes.

Known CI issues:

CI fail at windows-latest (4.1) is from main. Ref: https://github.com/r-lib/covr/actions/runs/6460571660/job/17538531691
CI fail at ubuntu-latest (oldrel-4) R-3.6 will be addressed in the next release of box

* Adjusted DESCRIPTION and NEWS.md * Different condition to skip tests * Bump version for release --------- Co-authored-by: Jim Hester <jimhester@netflix.com>

radbasa · 2023-10-11T04:17:49Z

Update 3dca46d fixed the CI issue with windows-latest (4.1).
~~CI fail at windows-latest (release) is a problem installing Ghostscript. This ran fine in our CI.~~

king-of-poppk · 2024-01-08T10:06:24Z

Hi @radbasa! What is currently blocking this PR?

radbasa · 2024-01-10T07:00:41Z

Hi @radbasa! What is currently blocking this PR?

Hi @king-of-poppk , I believe it's an R 3.6.3 backports issue. CI is failing on oldrel-4 which is Ubuntu 22.04.3 LTS-R version 3.6.3.

Specifically, the problem is when box 1.1.3 is being installed.

  ℹ Building box 1.1.3
  ✖ Failed to build box 1.1.3
  Error: 
  ! error in pak subprocess
  Caused by error in `stop_task_build(state, worker)`:
  ! Failed to build source package box.
  ---
  Backtrace:
  1. pak::lockfile_install(".github/pkg.lock")
  2. pak:::remote(function(...) { …
  3. err$throw(res$error)

An August 2023 commit on box should unblock this PR. We'll have to wait for @klmr to publish the next version of box to CRAN.

* `activeBindingFunction()` needs to reset the environment’s class after clearing it. * `tryInvokeRestart()` was missing

king-of-poppk · 2024-02-07T10:25:54Z

An August 2023 commit on box should unblock this PR. We'll have to wait for @klmr to publish the next version of box to CRAN.

@radbasa box 1.2.0 which includes klmr/box@9c47553 was just released.

radbasa · 2024-02-07T12:31:18Z

An August 2023 commit on box should unblock this PR. We'll have to wait for @klmr to publish the next version of box to CRAN.

@radbasa covr 1.2.0 which includes klmr/box@9c47553 was just released.

Thank you for the heads-up, I'll work on it. We'll have to wait for packagemanager.posit.co to serve box@1.2.0.

king-of-poppk · 2024-02-07T12:40:41Z

covr 1.2.0

@radbasa Sorry, meant box 1.2.0. Edited.

radbasa · 2024-02-08T03:03:18Z

Hi @jimhester , the backports issue with CI/CD oldrel-4 has been resolved by the release of box@1.2.0. All checks now pass. When you have the time, we would appreciate it if you take a look at this PR. Thank you!

jimhester · 2024-02-08T20:09:17Z

Thanks for the update, LGTM

radbasa and others added 26 commits January 19, 2023 14:25

increment version

01b1249

an app with box modules example

a19ffd0

boilerplate for box coverage test

e9fe6ff

ready to write box support

1a0c5b7

now works with basic box modules

337d1cc

Forgot an s

7d4a35a

Co-authored-by: Jakub Nowicki <q.nowicki@gmail.com>

Smaller version increment as suggested

d1f485c

Co-authored-by: Jakub Nowicki <q.nowicki@gmail.com>

Merge pull request #9 from Appsilon/feature/klmr-box-support

4212ad1

Basic klmr/box support

got it to work!

95b51d7

added support for R6 classes attached as box modules

938b838

increment dev version

8805e9d

forgot to update the unit test with R6 modules attached

9b9251a

Merge pull request #11 from Appsilon/feature/private-functions

700f505

Add covr visibility into private box module functions

Merge pull request #12 from Appsilon/feature/r6-objects

7fc8e31

Feature/r6 objects

some refactoring to clean up code

924fd06

tests for klmr/box attached modules, functions, three dots, and aliases

1020092

Merge branch 'feature/attached-functions-tests' into feature/code-cle…

a7853bd

…anup

a little bit of cleanup

e932fe3

code cleanup. finally broke the unlist puzzle

1ea85e1

clean up nested ((

2a9ab9b

Merge pull request #14 from Appsilon/feature/attached-functions-tests

f1eee66

Tests for box module attached functions, three dots, aliases

Merge pull request #16 from Appsilon/feature/code-cleanup

1a1f6b9

Feature/code cleanup

test: Fix box cache cleaning in tests.

513c051

chore: Add box to suggested dependencies.

0b9de23

chore: Update package version.

08562d7

Merge pull request #17 from Appsilon/pre-pr-cleanup

46fe3b0

Pre pr cleanup

Merge branch 'main' into klmr-box-support

373cb40

added entry in NEWS.md

a78f548

Trigger CICD

8ac1268

jakubnowicki and others added 2 commits October 10, 2023 12:43

Merge branch 'main' into klmr-box-support

1426c5b

radbasa and others added 2 commits October 11, 2023 11:57

Update to covr 3.6.3 (#23)

2238519

* Adjusted DESCRIPTION and NEWS.md * Different condition to skip tests * Bump version for release --------- Co-authored-by: Jim Hester <jimhester@netflix.com>

Merge branch 'main' into klmr-box-support

0d3351e

Merge branch 'main' into klmr-box-support

22ef30e

radbasa mentioned this pull request Oct 12, 2023

Consider making ‘box’ namespace environments actual namespaces klmr/box#341

Closed

Update to covr 3.6.4. Merge branch 'main' into klmr-box-support

216f027

king-of-poppk referenced this pull request in klmr/box Jan 10, 2024

Fix errors in R 3.6.3 backports

9c47553

* `activeBindingFunction()` needs to reset the environment’s class after clearing it. * `tryInvokeRestart()` was missing

box 1.2.0 released with backports compatibility fixes

b93fd1b

jimhester merged commit 3ec2edf into r-lib:main Feb 8, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

klmr/box support #526

klmr/box support #526

radbasa commented Mar 6, 2023 •

edited

Loading

CLAassistant commented Mar 6, 2023 •

edited

Loading

radbasa commented Mar 15, 2023

jimhester commented Apr 20, 2023

jakubnowicki commented Apr 21, 2023 •

edited

Loading

radbasa commented Oct 10, 2023 •

edited

Loading

radbasa commented Oct 11, 2023 •

edited

Loading

king-of-poppk commented Jan 8, 2024

radbasa commented Jan 10, 2024 •

edited

Loading

king-of-poppk commented Feb 7, 2024 •

edited

Loading

radbasa commented Feb 7, 2024 •

edited

Loading

king-of-poppk commented Feb 7, 2024

radbasa commented Feb 8, 2024 •

edited

Loading

jimhester commented Feb 8, 2024

klmr/box support #526

klmr/box support #526

Conversation

radbasa commented Mar 6, 2023 • edited Loading

What works:

Changes:

Not implemented:

CLAassistant commented Mar 6, 2023 • edited Loading

radbasa commented Mar 15, 2023

jimhester commented Apr 20, 2023

jakubnowicki commented Apr 21, 2023 • edited Loading

radbasa commented Oct 10, 2023 • edited Loading

Known CI issues:

radbasa commented Oct 11, 2023 • edited Loading

king-of-poppk commented Jan 8, 2024

radbasa commented Jan 10, 2024 • edited Loading

king-of-poppk commented Feb 7, 2024 • edited Loading

radbasa commented Feb 7, 2024 • edited Loading

king-of-poppk commented Feb 7, 2024

radbasa commented Feb 8, 2024 • edited Loading

jimhester commented Feb 8, 2024

radbasa commented Mar 6, 2023 •

edited

Loading

CLAassistant commented Mar 6, 2023 •

edited

Loading

jakubnowicki commented Apr 21, 2023 •

edited

Loading

radbasa commented Oct 10, 2023 •

edited

Loading

radbasa commented Oct 11, 2023 •

edited

Loading

radbasa commented Jan 10, 2024 •

edited

Loading

king-of-poppk commented Feb 7, 2024 •

edited

Loading

radbasa commented Feb 7, 2024 •

edited

Loading

radbasa commented Feb 8, 2024 •

edited

Loading