Skip to content

refactor: ♻️ convert column time suffixes to its own variable in VAS#89

Open
joelostblom wants to merge 7 commits into
mainfrom
feat/time-suffixes
Open

refactor: ♻️ convert column time suffixes to its own variable in VAS#89
joelostblom wants to merge 7 commits into
mainfrom
feat/time-suffixes

Conversation

@joelostblom
Copy link
Copy Markdown

Description

@martonvago Sorry this took longer than expected. Here is an attempt make changes to the script and the resulting differences in the datapackage.json file. All _XXmin suffixes are now removed and instead included as a variable. Don't hesitate to let me know if I missunderstood something from the issue.

Closes #45

This PR needs a quick/an in-depth review.

Checklist

  • Ran just run-all

@joelostblom joelostblom requested a review from a team as a code owner May 22, 2026 17:22
@joelostblom joelostblom moved this from Todo to In review in Data development May 22, 2026
Copy link
Copy Markdown
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

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

Nicely reduces the number of tables we have 🎉 just some comments and changes.

Comment thread scripts/redcap_dict_to_properties.py Outdated
Comment thread scripts/redcap_dict_to_properties.py Outdated
Comment thread scripts/redcap_dict_to_properties.py Outdated
Comment thread scripts/redcap_dict_to_properties.py Outdated
Comment thread scripts/redcap_dict_to_properties.py Outdated
Comment thread datapackage.json Outdated
Comment thread scripts/redcap_dict_to_properties.py Outdated
@github-project-automation github-project-automation Bot moved this from In review to In progress in Data development May 26, 2026
Comment thread scripts/redcap_dict_to_properties.py Outdated
Join vas_minus10 into the combined VAS resource using minutes_from_meal
so negative and positive meal-relative timepoints share one schema.
Remove the redundant vas_ prefix from variables
after they are moved into the combined VAS resource.
Use descriptive title and description values for the combined VAS resource and make VAS field deduplication easier to read.
Copy link
Copy Markdown
Author

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

Thanks for the comments; updated accordingly!

Comment thread scripts/redcap_dict_to_properties.py Outdated
Comment thread scripts/redcap_dict_to_properties.py Outdated
@joelostblom joelostblom moved this from In progress to In review in Data development May 27, 2026
Comment thread scripts/redcap_dict_to_properties.py
@lwjohnst86 lwjohnst86 changed the title refactor: ♻️ convert column time suffixes to its own variable refactor: ♻️ convert column time suffixes to its own variable in VAS May 28, 2026
@martonvago martonvago mentioned this pull request May 28, 2026
1 task
martonvago
martonvago previously approved these changes May 28, 2026
Copy link
Copy Markdown
Collaborator

@martonvago martonvago left a comment

Choose a reason for hiding this comment

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

🎉 👏👏👏

Copy link
Copy Markdown
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

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

Looking very nice, only two suggested changes (plus asking @K-Beicher about the title).


def _get_resource_title(form_name: str) -> str:
if form_name == "vas":
return "Visual analogue scale measurements"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@K-Beicher is this an accurate title for this resource?

Comment thread datapackage.json Outdated
"title": "hunger",
"type": "number",
"description": "Visual analogue scale. How hungry do you feel, at time minus 10 min Question: Hvor sulten er du?. Slider scale labels: Jeg er slet ikke sulten | | Jeg har aldrig været mere sulten",
"description": "Visual analogue scale. How hungry do you feel Question: Hvor sulten er du?. Slider scale labels: Jeg er slet ikke sulten | | Jeg har aldrig været mere sulten",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The "Visual analogue scale. " can be removed from these descriptions. Also, can you add a regex replacer to put a period before Question: if there is no period?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done in cf61caf

@github-project-automation github-project-automation Bot moved this from In review to In progress in Data development May 28, 2026
@joelostblom joelostblom moved this from In progress to In review in Data development May 28, 2026
@joelostblom joelostblom requested a review from lwjohnst86 May 28, 2026 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Need to convert VAS resources into one resource

3 participants