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.

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

update packages versions for scheduled build #330

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

m7pr
Copy link
Contributor

@m7pr m7pr commented Jan 21, 2025

@m7pr m7pr added the core label Jan 21, 2025
Copy link
Contributor

github-actions bot commented Jan 21, 2025

Unit Tests Summary

 1 files   1 suites   58s ⏱️
 6 tests  6 ✅ 0 💤 0 ❌
12 runs  12 ✅ 0 💤 0 ❌

Results for commit 2d67b57.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 21, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_g_gh_boxplot 💚 $58.08$ $-1.09$ $0$ $0$ $0$ $0$

Results for commit e034671

♻️ This comment has been updated with latest results.

@m7pr
Copy link
Contributor Author

m7pr commented Jan 21, 2025

2 dependency tests fixed. Updating testthat to fix the 3rd one

https://github.com/insightsengineering/teal.goshawk/actions/runs/12883188382

Comment on lines 41 to 42
extra-deps: |
MultiAssayExperiment (>= 1.32.0);SummarizedExperiment (>= 1.36.0);teal.slice (>= 0.5.1.9021)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is interesting as teal.goshawk has nothing to do with MAE and SE packages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gogonzo extra-deps is the parameter for packages that have nothing to do with the original package.
extra-deps is for the second or for the third level dependencies

teal.goshawk -> teal -> teal.slice -> MultiAssayExperiment, SummarziedExperiment

If you want to specify min version of a 3rd level dependency and you don't want to put it inside DESCRIPTION file, this is the way to go. We discussed this with @pawelru here insightsengineering/teal#1445 (comment)

@m7pr
Copy link
Contributor Author

m7pr commented Jan 22, 2025

Specified shinytest2 version and rerunning the build https://github.com/insightsengineering/teal.goshawk/actions/runs/12908665973

@m7pr
Copy link
Contributor Author

m7pr commented Jan 22, 2025

@m7pr m7pr requested a review from llrs-roche January 23, 2025 08:06
@m7pr
Copy link
Contributor Author

m7pr commented Jan 23, 2025

@llrs-roche would you mind taking a look? I fixed all dependency tests (besides release ofc).

@llrs-roche llrs-roche self-assigned this Jan 23, 2025
Copy link

@llrs-roche llrs-roche left a comment

Choose a reason for hiding this comment

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

Looks good, just one comment regarding the Bioconductor dependencies.

I am also surprised that testthat 3.20 works here instead of requiring 3.2.2 for the problems we had with rlang and withr in other checks.

.github/workflows/scheduled.yaml Outdated Show resolved Hide resolved
@@ -38,6 +38,8 @@ jobs:
strategy: ${{ matrix.test-strategy }}
additional-env-vars: |
PKG_SYSREQS_DRY_RUN=true
extra-deps: |
matrixStats (>= 1.5.0);teal.slice (>= 0.5.1.9021)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we may be able to remove teal.slice from here, once this is merged https://github.com/insightsengineering/teal/pull/1461/files

@m7pr m7pr changed the title WIP update packages versions for scheduled build update packages versions for scheduled build Jan 23, 2025
@m7pr
Copy link
Contributor Author

m7pr commented Jan 23, 2025

Rerunning after the relaxation of assumptions on bioconductor packages versions https://github.com/insightsengineering/teal.goshawk/actions/runs/12932596927

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants