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

Stable release channel for TLG catalog #122

Merged
merged 41 commits into from
Oct 5, 2023
Merged

Stable release channel for TLG catalog #122

merged 41 commits into from
Oct 5, 2023

Conversation

walkowif
Copy link
Contributor

@walkowif walkowif commented Sep 19, 2023

In order to successfully build the TLG Catalog for stable package versions, I had to temporarily remove some contents of files in book/tables/pharmacokinetic.

Before merging we'll have to figure out what to do about it.

Two TLG Catalog versions (development and stable) deployed concurrently can be viewed here (unless they were overwritten by a scheduled job).

This PR also adds ideas from https://github.com/insightsengineering/idr-tasks/issues/581.

@walkowif walkowif requested a review from a team September 25, 2023 12:20
@walkowif walkowif marked this pull request as ready for review September 25, 2023 12:22
@github-actions
Copy link
Contributor

github-actions bot commented Sep 25, 2023

Unit Tests Summary

    1 files  2 suites   1m 1s ⏱️
  26 tests 0 ✔️   26 💤 0
281 runs  0 ✔️ 281 💤 0

Results for commit 716223a.

♻️ This comment has been updated with latest results.

@cicdguy
Copy link
Contributor

cicdguy commented Sep 28, 2023

In order to successfully build the TLG Catalog for stable package versions, I had to temporarily remove some contents of files in book/tables/pharmacokinetic.
Before merging we'll have to figure out what to do about it.

What was the issue with these?

book/_quarto-development.yml Outdated Show resolved Hide resolved
book/_quarto-stable.yml Outdated Show resolved Hide resolved
@walkowif
Copy link
Contributor Author

In order to successfully build the TLG Catalog for stable package versions, I had to temporarily remove some contents of files in book/tables/pharmacokinetic.
Before merging we'll have to figure out what to do about it.

What was the issue with these?

There were various errors, for example:

https://github.com/insightsengineering/tlg-catalog/actions/runs/6297102070/job/17093397369

Error in `format_sigfig()`:
! could not find function "format_sigfig"
Backtrace:
 1. ... %>% ...
 2. tern::analyze_vars_in_cols(...)

https://github.com/insightsengineering/tlg-catalog/actions/runs/6296594726/job/17091890939

! error in evaluating the argument 'obj' in selecting a method for function 'next_rpos': error in evaluating the argument 'obj' in selecting a method for function 'next_rpos': unused argument (alt_counts = TRUE)
Backtrace:
 1. ... %>% ...
 8. base::.handleSimpleError(...)
 9. base (local) h(simpleError(msg, call))

I think they might be related to some NEST package features used by TLG Catalog being unavailable in package versions released to https://insightsengineering.r-universe.dev/.

@cicdguy
Copy link
Contributor

cicdguy commented Sep 28, 2023

I think they might be related to some NEST package features used by TLG Catalog being unavailable in package versions released to https://insightsengineering.r-universe.dev/.

Yeah that makes sense. @insightsengineering/nest-sme - can we get some insight into these errors when you guys get a chance? We can provide more details if needed.

@edelarua
Copy link
Contributor

@cicdguy @walkowif,

Both errors are caused by updates made in tern 0.9.0.9002 (this PR). Would this be fixed after the upcoming tern 0.9.1 release?

@cicdguy
Copy link
Contributor

cicdguy commented Sep 28, 2023

Would this be fixed after the upcoming tern 0.9.1 release?

Very likely that it'll be fixed following the release, yes. We'll just wait till the next release before pushing this out. Thanks, @edelarua!

@walkowif
Copy link
Contributor Author

walkowif commented Oct 5, 2023

Now we have this error when generating the TLG Catalog for stable package versions:

Quitting from lines 81-107 [variant2] (mmrmt01.qmd)
Error:
! 'free_cores' is not an exported object from 'namespace:mmrm'
Backtrace:
 1. tern.mmrm::fit_mmrm(...)
 7. base::ifelse(parallel, mmrm::free_cores(), 1L)
Execution halted

@edelarua
Copy link
Contributor

edelarua commented Oct 5, 2023

Now we have this error when generating the TLG Catalog for stable package versions:

Quitting from lines 81-107 [variant2] (mmrmt01.qmd)
Error:
! 'free_cores' is not an exported object from 'namespace:mmrm'
Backtrace:
 1. tern.mmrm::fit_mmrm(...)
 7. base::ifelse(parallel, mmrm::free_cores(), 1L)
Execution halted

@walkowif it seems like the Statistical Engineering team has replaced this function in mmrm v0.3.3 but has not yet released tern.mmrm v0.2.3 (latest is v0.2.2.9006) which accounts for this change in mmrm. I'm not sure when the next planned release for tern.mmrm is (it was an uncommitted item on Kendis).

@cicdguy
Copy link
Contributor

cicdguy commented Oct 5, 2023

Let's leave this qmd out for now (just rename it to .qmd.refactor-pending or something).

@walkowif
Copy link
Contributor Author

walkowif commented Oct 5, 2023

Eventually, these two files had to be renamed:

  • book/graphs/efficacy/mmrmg02.qmd.refactor-pending
  • book/tables/efficacy/mmrmt01.qmd.refactor-pending

@cicdguy
Copy link
Contributor

cicdguy commented Oct 5, 2023

Okay we'll revisit these 2 later. Let's get this merged for now

@cicdguy cicdguy merged commit a5e0d83 into main Oct 5, 2023
19 checks passed
@cicdguy cicdguy deleted the stable-tlg-catalog branch October 5, 2023 20:19
@danielinteractive
Copy link

So now that mmrm is released on CRAN and tern.mmrm is fixed, this should be resolvable

@imazubi
Copy link

imazubi commented Nov 21, 2023

FYI, the MMRM module from the teal.gallery is showing some errors as well. I tried out with different user input values.

image

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

Successfully merging this pull request may close these issues.

5 participants