-
Notifications
You must be signed in to change notification settings - Fork 5
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
New approach #55
Merged
Merged
New approach #55
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- see news
removing version from is_installed to fix bug introduced with new R version
fixing test issue caused by converting covariateSettings to a list
see news
fixing checks
fixing example check issues
- removed pdf from package as user can access via website - updated website - renamed vignette called UsingCharacterizationPackage to UsingPackage as name too long for window's R check
updating aggregate characterization to use custom during covariates for case analysis
…inations.sql Fixed case sensitivity
Changed case of CreateTargetOutcomeCombinations.sql
…terization into improving_aggregate
fixing primary key for test to pass
Improving aggregate results model
removing migration and package_version from exports
- added columns to PK for dechal-rechal table - added max rows to export input when doing csv export
- supporting multiple TARs - split aggregate covariate SQL into parts with and without TAR - added time_to_event result table with migrations
fixing time_at_risk tables
- adding checks for unique cohort_definition_ids
- using row_id for FE to not have conflicts - adding hash of settings to be runId for uniqueness and for incremental mode
- runs codes in parallel - has incremental mode - saves direct to csv and then aggregates csvs
- fixing tte and dcrc to export csv files - fixing bug when no results for aggregate covariates - adding tests to get 90% coverage
updating vignette
removing primary key from counts as columns in key can be NULL
- putting Os into T aggregate extraction for efficiency
- created cleaner runCharacterization using do.call - updated cohortTypes for aggregate covariates to be more intuitive - improved incremental - made result folder location more flexible - added parallelization to tte and dechal-rechal - TODO: update for new FE and update aggregate cov job to not run target if desired
- updating min FE version and adding the two columns that new FE requires in covariate_ref - making the non-case aggregate covariate setting work and adding a test for it
- adding log - fixing small bug
removing . in documentation
minor fixes
- added check to ensure single tar per aggregate setting
- adding manual data test - fixed but in target counts found with manual test
- added test to manual data to ensure cohort type Exclude works
- editing setting_id to be float - moving tables/columns in resulttables.sql to be consistent with resultsdatamodelspecifications.csv
- adding random file location to prevent OS test clashes
- updating docs
- adding time delay to ensure setting ids are unique between target and cases
fixing issue with different case tars having same id
- adding edits based on commit to old Characterization by Anthony Sena
- removed code from dechal-rechal to return rows when no dechal as it is much faster - updated tests - fixed missing DatabaseConnection:: in db tests
fixing db tests
- fixing bug where mincellcount wasnt being used by aggregate covs - adding tests for all mincellcount cells - turning off GHA mac spark tests due to odd java error on there (tested manually on my mac)
- updating docs - turning off dbms test except for windows
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before you do a pull request, you should always file an issue and make sure the package maintainer agrees that it’s a problem, and is happy with your basic proposal for fixing it. We don’t want you to spend a bunch of time on something that we don’t think is a good idea.
Additional requirements for pull requests:
Adhere to the Developer Guidelines as well as the OHDSI Code Style.
If possible, add unit tests for new functionality you add.
Restrict your pull request to solving the issue at hand. Do not try to 'improve' parts of the code that are not related to the issue. If you feel other parts of the code need better organization, create a separate issue for that.
Make sure you pass R check without errors and warnings before submitting.
Always target the
develop
branch, and make sure you are up-to-date with the develop branch.