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

Support more MAE and SE versions in CI Pipelines #637

Merged
merged 1 commit into from
Jan 23, 2025
Merged

Support more MAE and SE versions in CI Pipelines #637

merged 1 commit into from
Jan 23, 2025

Conversation

m7pr
Copy link
Contributor

@m7pr m7pr commented Jan 22, 2025

A follow-up after #634 that allows to use older MAE and SE versions

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

m7pr commented Jan 22, 2025

Copy link
Contributor

github-actions bot commented Jan 22, 2025

Unit Tests Summary

  1 files   29 suites   27s ⏱️
369 tests 369 ✅ 0 💤 0 ❌
821 runs  821 ✅ 0 💤 0 ❌

Results for commit a8510fa.

♻️ This comment has been updated with latest results.

@m7pr
Copy link
Contributor Author

m7pr commented Jan 22, 2025

Hi, cc @llrs-roche, @gogonzo, and @donyunardi, as you previously expressed interest in the recent updates to fixing our pipelines.

Initially, I thought we needed to specify higher version requirements for MultiAssayExperiment and SummarizedExperiment (which are listed in Suggests) to ensure the Scheduled job scenarios could install all dependencies without issues. This was because one of the dependencies, matrixStats, was causing a crash. However, I later realized we only need to specify this third-level dependency (matrixStats) to be built from the latest Bioconductor release. This allows us to avoid version-locking MultiAssayExperiment and SummarizedExperiment, keeping them compatible with older versions of those packages as well.

Since this approach worked for teal.slice, I applied the same reduction of version requirements to teal and tmg. I’ll also make similar changes to tmc and other teal.modules.* packages.

Here are the relevant pull requests:

On a personal note, I find this to be an unnecessary complication since MultiAssayExperiment and SummarizedExperiment are only listed in Suggests.

@donyunardi
Copy link
Contributor

On a personal note, I find this to be an unnecessary complication since MultiAssayExperiment and SummarizedExperiment are only listed in Suggests.

We need to start looking into this issue:

Let's do it this year.

m7pr added a commit to insightsengineering/teal that referenced this pull request Jan 23, 2025
Tries to reduce some package versioning complexity in CI pipelines. More
explanation in here
insightsengineering/teal.slice#637 (comment)
@llrs-roche llrs-roche self-assigned this Jan 23, 2025
Copy link
Contributor

@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.

The change makes sense and a the checks remained with the same failure as main on this branch

m7pr added a commit to insightsengineering/teal.modules.general that referenced this pull request Jan 23, 2025
Companion to that allows to use older MultiAssayExperiment and older
SummarizedExperiment
More explanation in here
insightsengineering/teal.slice#637 (comment)

insightsengineering/teal#1460
insightsengineering/teal.slice#637
@m7pr m7pr merged commit bfaa4fc into main Jan 23, 2025
36 of 37 checks passed
@m7pr m7pr deleted the ci_pipelines branch January 23, 2025 14:20
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants