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

tie together the various constants used for parsing the redcap CSV #54

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

alistairewj
Copy link
Collaborator

I refactored the plethora of hard-coded dictionaries we had into a single Enum with an Instrument class. The goal of this was to (1) tie together all these constants so we wouldn't forget to add a columns variable for a questionnaire with the name variable, (2) reduce the number of hard-coded values, and (3) reduce the complexity of multiple dictionaries interacting.

  • The Instrument class contains the hard-coded constants we need for each type of redcap instrument, e.g. the session_id which unique identifiers it, the name of the JSON file with the columns, and the name of the FHIR schema that was automatically converted from ReproSchema
  • I fixed a few typos in the name of the questionnaires
  • I now impute the redcap_instrument column in the loaded CSV (so I renamed this to be specific to loading a redcap CSV). This makes it easier for the later code to filter rows/columns together, since we can rely on the redcap_instrument column always having a value
  • Cleaned up the interface to the dataset class a bit. Now the find_* methods return a list of Path objects, no more awkward dictionary stuff. This also fixed a bug where multiple sessions for a participant were merged together implicitly. It's a better signature in general IMO.
  • Removed a lot of the now redundant list_* methods
  • Updated the tutorial and added some code on loading audio

Copy link
Contributor

@ibevers ibevers left a comment

Choose a reason for hiding this comment

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

This look a lot better! I'm glad that this also led to some bugs getting fixed

@ibevers ibevers merged commit b6351f0 into main Jun 12, 2024
2 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.

3 participants