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

429 save cdm database name #430

Merged
merged 9 commits into from
Mar 18, 2024
Merged

429 save cdm database name #430

merged 9 commits into from
Mar 18, 2024

Conversation

egillax
Copy link
Collaborator

@egillax egillax commented Mar 1, 2024

Fixes #429

Copy link

codecov bot commented Mar 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.44%. Comparing base (f212580) to head (56b5f65).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #430      +/-   ##
===========================================
+ Coverage    89.32%   89.44%   +0.12%     
===========================================
  Files           49       49              
  Lines         9830     9838       +8     
===========================================
+ Hits          8781     8800      +19     
+ Misses        1049     1038      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@egillax
Copy link
Collaborator Author

egillax commented Mar 6, 2024

@jreps this should be now ready for review. I've also tested it locally by fitting two models and viewing them in the shiny since I was worried this might affect that.

I had to make a few other changes to get the tests to pass. Was dealing with some strange test failures that only happened during code coverage in github actions.

  • I moved the codecov calculations to the linux runner - for me that one is always faster.
  • In response to FeatureExtraction's deprecation of cohortId in favour of cohortIds I had to change getCohortCovariateData to take in cohortIds since it's called through FeatureExtraction::getDbCovariateData
  • Due to roxygen 7.3 I changed @docType package in PatientLevelPrediction.R is deprecated so I changed that to the recommended approach.

@egillax egillax requested a review from jreps March 6, 2024 09:20
@egillax egillax merged commit 11122d4 into develop Mar 18, 2024
10 checks passed
egillax added a commit that referenced this pull request Apr 5, 2024
* Update DESCRIPTION

* only resrtict to first if many observations per subjectId

* fix assignment operator in configurePython (#421)

* Tibble dependancy removal (#422)

* remove unnecessary remotes (#423)

* Study population improvements (#424)

* assign population if existing and added a test (#428)

* 429 save cdm database name (#430)

* save dev database name and schema in trainDetails (#434)

* preserve attributes when splitting data

* Prevent plpData from being evaluated during do.call (#436)

* test improvements (#438)

* fix duplicate vignette titles

---------

Co-authored-by: jreps <[email protected]>
Co-authored-by: Henrik <[email protected]>
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.

1 participant