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

V2.0 release candidate #52

Merged
merged 100 commits into from
Apr 23, 2024
Merged

V2.0 release candidate #52

merged 100 commits into from
Apr 23, 2024

Conversation

ablack3
Copy link
Collaborator

@ablack3 ablack3 commented Jun 21, 2023

No description provided.

msuchard and others added 30 commits January 12, 2021 07:12
Add test for exportToCsv
skip tests when Java 8 is not available
…supportsJava8() function during tests. (#15)

Co-authored-by: Adam Black <[email protected]>
…on, add installation of libcurl and libssh to github actions workflow.
Remove dependency on rcmdcheck, remove test of supportsJava8() function, add installation of libcurl and libssh to github actions workflow.
)

* Use \dontrun{} on all examples so they will not be executed on CRAN servers.

* Wrap more examples with dontrun{}
* removed drat installation instructions, CRAN only

* Update README.md

* updates and fixes

switched to readr for csv writing fixes #13
added gender concepts to concept table fixes #21 fixes #19

Co-authored-by: Frank DeFalco <[email protected]>
Co-authored-by: Martijn Schuemie <[email protected]>
Co-authored-by: Frank DeFalco <[email protected]>
Fix GiBleed.sql
… translate calls because it causes cryptic errors when devtools::test() is called.
…8 function and RSQLite is actually not used by this package directly so it is removed from Description to avoid the R check NOTE.
Reads the environment variable to get the github base url to EunomiaDatasets.

Downloads and specified dataset and version as a zip file.

Reads in all the CSVs.
@ginberg
Copy link
Collaborator

ginberg commented Aug 28, 2023

hi @fdefalco, I am wondering if you already had a look at this PR?

All tests and checks pass.
Added support for parquet
Supports CDM 5.3 and 5.4 confirmed.
Achilles runs against resulting data sets. (requires updated develop branch of achilles)
@fdefalco
Copy link
Collaborator

I've pushed a number of new changes to this branch so it now has support for duckdb, sqlite, with csv and parquet files. It is failing in the runner due to an issue with a remotes dependency which I'll try and figure out. If you can each give this a test it would be much appreciated! @ablack3 @ginberg

#' @param overwrite Remove and replace an existing data set.
#' @export
loadDataFiles <- function(dataPath,
dbPath,
Copy link
Collaborator

Choose a reason for hiding this comment

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

the function parameters are not properly aligned

@ginberg
Copy link
Collaborator

ginberg commented Sep 26, 2023

hi @fdefalco, thanks for the updates! Overall it's working well for me. But, we have a 'Remotes' section in the Description now for the CommonDataModel, which means that we can't submit it to CRAN. This is a pity because we would also like to put other packages depending on Eunomia, like FeatureExtraction, on CRAN. We are just using one method from CommonDataModel it seems (writeDdl). Maybe we can include this function (for now) into this package and have a long term goal of also putting the CommonDataModel on CRAN?

@fdefalco
Copy link
Collaborator

fdefalco commented Oct 3, 2023

Ok. I'm going to work on getting CommonDataModel into CRAN, then it won't be a remotes dependency.

@anthonysena
Copy link
Collaborator

Just wanted to follow up here to see if there has been any progress towards getting the CommonDataModel package into CRAN @fdefalco? I also wanted to ask why we've introduced this dependency into Eunomia? With Eunomia 2.0, we've externalized the data sets and have versioned them according to the CDM specification (i.e. v5.3, v.5.4) so the DDL that is created can be implied from the .csv files in that data. If Eunomia is geared towards providing a lightweight testing DB, is this not sufficient?

@fdefalco
Copy link
Collaborator

Happy to report that CommonDataModel has now been approved to CRAN so re-initiating work on this thread.

https://cran.r-project.org/web/packages/CommonDataModel/index.html

@anthonysena
Copy link
Collaborator

Following up here @fdefalco - just let me know if I can help to resolve these conflicts so we can test & release Eunomia 2.0.

@anthonysena
Copy link
Collaborator

Noticed the activity here - thanks for pushing this along @fdefalco. I also noticed the test failure on MacOS and it may be related to this issue: apache/arrow#41050.

@fdefalco fdefalco merged commit 79c8944 into main Apr 23, 2024
8 checks passed
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.

9 participants