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?

I suspect it will be easier to add a 'completed date' field to those that have a missing obvious value. I'd be happy to sit down and go through all the ones that are falling outside the norm.

For some forms a visit or date doesn't make a lot of sense, adverse events for instance, it doesn't really matter when the information was recorded, what everyone will be interested in is when the participant experienced the discomfort. But we can add an internal date to the form so that it captures when the information was recorded.

With the exception of the repeated instruments (like Adverse Events) all data in REDCap will be uniquely identifiable based on the record_id (which here is the participant ID) and the field name. If you have those two things you will be able to say exactly where that piece of information belongs.

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.

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?

Having looked at events in more detail, we have 21 with the following rough breakdown:

prescreening on the phone

10 visits, each tied to a date

planned and ad-hoc HbA1c measurements

Stamdata collection (adverse events link here)

3 phases that seem to span multiple weeks, each with group meetings and consultations in different weeks

info from unplanned “extra” contact

medicine changes

randomisation

So it looks like an event sometimes has a concrete date (like visits) and sometimes it doesn't (like phases). But individual forms usually have a date field (date of adverse event, date of group meeting, date of medicine change, etc.). However, it's not always obvious which field should be used as the "real" date field because they all have different names and some forms have multiple date fields.

We were also wondering which events have forms that contain medical/useful info (not just admin). Having looked at the fields, I'd say that all of them contain medical data (usually mixed with admin), except for randomisation.

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.

martonvago · 2026-06-15T15:23:29Z

I undid updating the data dict in this PR because it should go in a separate one together with updating the data.

lwjohnst86

I think we should go with @joelostblom suggestion and wrangle the data first and then extract the metadata and connect what we can with the data dictionary from REDCap. The primary key and connected foreign key aren't well established enough for us to effectively take the REDCap data and get it into the correct resources and format.

I think that we should also be creating some doc to describe how the flow is exactly working between REDCap and the data package, because right now it isn't clear and we're having to do a lot back and forth discussions and clarifications. Those back and forths should be handled in docs. I'll make an issue about that.

For now, I'll convert this into a draft and move into in-progress.

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

Comment thread scripts/redcap_dict_to_properties.py Outdated

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

martonvago mentioned this pull request Jun 15, 2026

feat: ✨ stage basic raw data #108

Draft

1 task

revert: ⏪ undo updating data dict

305c870

lwjohnst86 reviewed Jun 22, 2026

View reviewed changes

lwjohnst86 marked this pull request as draft June 22, 2026 13:02

lwjohnst86 moved this from In review to In progress in Data development Jun 22, 2026

Uh oh!

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!

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!

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!

martonvago commented Jun 15, 2026

Uh oh!

lwjohnst86 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

martonvago commented Jun 10, 2026 •

edited

Loading

martonvago Jun 10, 2026 •

edited

Loading