You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Achilles has been invited to join the HADES collection of packages. I would like to see the package included in HADES and will be creating a project to track the necessary tasks. Here are some of the things that would be needed to make Achilles HADES-compatible as detailed by @schuemie :
The master branch should contain the latest released version, so if people do remotes::install_github() they get the latest released version, not some development version.
Each version should be tagged in GitHub (e.g. ‘v1.6.7’). The tag should correspond to the version in the DESCRIPTION file. (This allows people (and renv) to install specific older versions using remotes::install_github(“ohdsi/Achilles”, ref = “v1.6.7”) ).
Pull requests and all development should take place in a develop branch. (The automated release process is triggered by merging develop in to master) That will guarantee that (a) the package build was successful, and (b) that the tag version matches the DESCRIPTION version. (it also pushes the new version to drat and BroadSea).
In general just adhering to the developer guidelines, code style guide, etc. as posted on https://ohdsi.github.io/Hades/, although I think for the most part Achilles already does.
Achilles for most people is the first OHDSI package they’ll encounter. But for a new user, this seems way too complex: https://ohdsi.github.io/Achilles/articles/RunningAchilles.html. Most users probably just need a single function that runs Achilles and produces the JSON files. I would put multithreading under an ‘advanced topics’ header.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
-
Achilles has been invited to join the HADES collection of packages. I would like to see the package included in HADES and will be creating a project to track the necessary tasks. Here are some of the things that would be needed to make Achilles HADES-compatible as detailed by @schuemie :
The master branch should contain the latest released version, so if people do remotes::install_github() they get the latest released version, not some development version.
Each version should be tagged in GitHub (e.g. ‘v1.6.7’). The tag should correspond to the version in the DESCRIPTION file. (This allows people (and renv) to install specific older versions using remotes::install_github(“ohdsi/Achilles”, ref = “v1.6.7”) ).
The tag (and release) should be automatically generated by a GitHub Action (see https://github.com/OHDSI/EvidenceSynthesis/blob/master/.github/workflows/R_CMD_check_Hades.yaml#L128)
Pull requests and all development should take place in a develop branch. (The automated release process is triggered by merging develop in to master) That will guarantee that (a) the package build was successful, and (b) that the tag version matches the DESCRIPTION version. (it also pushes the new version to drat and BroadSea).
Release notes should be in NEWS.md (see for example https://github.com/OHDSI/EvidenceSynthesis/blob/master/NEWS.md). That way they’ll show up on the GitHub Pages generated by pkgdown.
The README file should adhere to the HADES template. (E.g https://github.com/OHDSI/EvidenceSynthesis/blob/master/README.md)
In general just adhering to the developer guidelines, code style guide, etc. as posted on https://ohdsi.github.io/Hades/, although I think for the most part Achilles already does.
@schuemie personal preferences:
Achilles for most people is the first OHDSI package they’ll encounter. But for a new user, this seems way too complex: https://ohdsi.github.io/Achilles/articles/RunningAchilles.html. Most users probably just need a single function that runs Achilles and produces the JSON files. I would put multithreading under an ‘advanced topics’ header.
Connections are precious (it seems we’re allowed only 2000 on our RedShift? And Oracle has a hard limit) A connection is opened and closed 15 times in one function here: https://github.com/OHDSI/Achilles/blob/master/R/Achilles.R. I would open once and reuse. And put the disconnect in an on.exit() clause so it is always executed. (e.g. https://github.com/OHDSI/CohortMethod/blob/master/R/DataLoadingSaving.R#L153 )
Killing the user’s loggers seems bad form (see https://github.com/OHDSI/Achilles/blob/master/R/Achilles.R#L105). Instead, I recommend just adding a file logger (and error report logger), as well as closing those loggers on exit (see for example https://github.com/ohdsi-studies/ScyllaEstimation/blob/master/R/Main.R#L93-L96 )
Beta Was this translation helpful? Give feedback.
All reactions