refactor: :recycle: update shared primary keys by martonvago · Pull Request #106 · onlimit-study/feasibility-data

martonvago · 2026-06-10T15:26:08Z

Description

This PR updates the primary keys common to all resources.
Based on my understanding of the situation and our discussions, but very open to comments.
I tried to include all my assumptions.

Closes #95

This PR needs an in-depth review.

Checklist

Ran just run-all

martonvago · 2026-06-10T15:30:49Z

+        description="The unique identifier of the participant.",
+        constraints=sp.ConstraintsProperties(required=True),
+    )
    event_field = sp.FieldProperties(


This is still the unique name of the event that we get from the raw data. There is also a numeric event ID, but we would need to look that up separately using another API request.

As for the relationship between events and visits, my understanding is that some events belong to a visit and some don't. So I don't know if adding a visit_id field to all resources makes sense because sometimes this will be empty. There are also forms with consult or week in the name, so that could be equivalent to visit. Maybe event is a broader category (any interaction with the participant) and visit is a particular type of interaction.

A visit ID should never be empty tho 🤔 Or at least in very few exceptions, like things collected between visits like the adverse events, but that will still have date fields that can line up with the visits. At the least, there should be a visit date.

What field does this relate to? Would we rename from another field? If so, would be good to put as a comment above the field variable.

This is from the raw redcap_event_name field, which is given for each row of the raw data. Event is the concept REDCap forms are organised around, visit is one kind of event in this study.

Visit ID can be missing if the row doesn't come from an instrument/form that is associated with a visit. I found about 24 forms like this. I looked at the fields for each to see if there is a date. There are date fields in each (e.g., contact_date, date_dietary_deviation_1, date_diet_consult_1), and some are also clearly associated with a study week or with another type of event (e.g., a consultation or a group meeting).

So we could pull out a date from each form, but it could be a bit faffy because we would need to check which field has the date in each. And then for forms that are associated with a visit, we would need to find the correct visit (based on the form name, event name, or field names) and then find the date for that visit. What do you think?

martonvago · 2026-06-10T16:14:05Z

    )
-    default_fields = [event_field, center_field]
-    primary_key = ["event"]
+    submission_field = sp.FieldProperties(


This is renamed from redcap_repeat_instance. Not all forms can repeat (= be submitted multiple times for the same participant and the same event), but when they do repeat, this number tells apart the submissions. I put it as a string to match the other IDs.

We can either add this everywhere and set it to 1 where the form doesn't repeat, or add it only for forms that can repeat.

Yea, I think only for forms that repeat.

Okay, I made it like that!
I included downloading the list of repeating instruments in the metadata download, so I can check which ones repeat.

martonvago · 2026-06-10T16:21:40Z

+        constraints=sp.ConstraintsProperties(required=True),
+    )
+    default_fields = [participant_field, event_field, submission_field, center_field]
+    primary_key = ["participant_id", "event_id", "submission_id"]


I didn't add an equivalent to redcap_repeat_instrument here because that should be the resource itself. When we transform the resources (like with VAS), we derive an extra PK component from the original instrument/form name, so that should maintain the distinction between rows coming from different instruments/forms.

lwjohnst86

Nice! Some questions ☺️

lwjohnst86 · 2026-06-11T07:29:02Z

@@ -234,8 +241,15 @@ def _form_to_resource(
            enum=["Copenhagen", "Aarhus", "Odense"],
        ),
    )


This field still needs to be more clear what it actually is. How does it relate to visit? Could you try to expand the description to make it clearer what it means? Maybe we can also rename it something clearer 🤔

My understanding is that an event is a collection of forms/instruments filled in at the same time. I think an event usually corresponds to a patient interaction (e.g., visit, consultation, group meeting, phone call), but it can also be more abstract (e.g., randomisation). So all visits are events but not all events are visits. I think this is why we initially decided not to rename event to visit, but there may be a better name.

@K-Beicher, is this correct?

lwjohnst86 · 2026-06-11T07:32:15Z

+        description="The unique identifier of the participant.",
+        constraints=sp.ConstraintsProperties(required=True),
+    )
    event_field = sp.FieldProperties(


A visit ID should never be empty tho 🤔 Or at least in very few exceptions, like things collected between visits like the adverse events, but that will still have date fields that can line up with the visits. At the least, there should be a visit date.

lwjohnst86 · 2026-06-11T07:32:59Z

    )
-    default_fields = [event_field, center_field]
-    primary_key = ["event"]
+    submission_field = sp.FieldProperties(


Yea, I think only for forms that repeat.

lwjohnst86 · 2026-06-11T07:35:22Z

+        description="The unique identifier of the participant.",
+        constraints=sp.ConstraintsProperties(required=True),
+    )
    event_field = sp.FieldProperties(


What field does this relate to? Would we rename from another field? If so, would be good to put as a comment above the field variable.

martonvago · 2026-06-11T14:24:00Z

Updated all metadata when I downloaded the list of repeating instruments

martonvago · 2026-06-11T14:44:24Z

If we add more requests, it will probably be worth generalising some logic here.

refactor: ♻️ update shared primary keys

dc57956

martonvago self-assigned this Jun 10, 2026

add-to-board-token Bot added this to Data development Jun 10, 2026

github-project-automation Bot moved this to Todo in Data development Jun 10, 2026

refactor: ♻️ remove extra brackets

061df51

martonvago commented Jun 10, 2026

View reviewed changes

Merge branch 'main' into refactor/primary-key

90793a4

martonvago moved this from Todo to In review in Data development Jun 10, 2026

martonvago marked this pull request as ready for review June 10, 2026 16:30

martonvago requested a review from a team as a code owner June 10, 2026 16:30

lwjohnst86 requested changes Jun 11, 2026

View reviewed changes

github-project-automation Bot moved this from In review to In progress in Data development Jun 11, 2026

martonvago and others added 3 commits June 11, 2026 14:50

refactor: ♻️ submission field only for repeating instruments

48bc601

feat: ✨ download repeating instruments and update data dict

55c6434

refactor: ♻️ regenerate datapackage.json

940d768

martonvago commented Jun 11, 2026

View reviewed changes

martonvago moved this from In progress to In review in Data development Jun 11, 2026

martonvago requested a review from lwjohnst86 June 11, 2026 14:47

Conversation

martonvago commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

martonvago Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lwjohnst86 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

martonvago commented Jun 10, 2026 •

edited

Loading

martonvago Jun 10, 2026 •

edited

Loading