refactor: ♻️ join SEFNC weeks resources by joelostblom · Pull Request #90 · onlimit-study/feasibility-data

joelostblom · 2026-05-27T10:23:51Z

Merge after #89

Description

This joins the SEFNC visit-specific resources into a single sefnc resource instead of having multiple resources with _vN suffixes.

I opted to repeat some of the vas_ functions here, because it didn't seem like much was gained in generalizing already. See #91 for details.

Closes #46

This PR needs an in-depth review.

Checklist

Ran just run-all

Normalize the SEFNC visit-specific REDCap forms into one resource schema with a week field so repeated measurements are represented by rows instead of suffixed columns.

Update the generated data package metadata so SEFNC has one resource with week values 0, 12, and 52 and no visit-suffixed duplicate fields.

joelostblom · 2026-05-27T10:25:30Z

+    "sefnc_baseline_v4": 0,
+    "sefnc_week12_v6": 12,
+    "selfefficacy_for_nutrition_change_sefnc_week_52": 52,
+}


I went with the week values 0 (baseline), 12, and 52 and removed the suffixes _v6 and _v10. Initially I thought that the suffixes would be the weeks, but the don't line up with the description, so maybe they stand for something like "visit" instead (but maybe it should have been a "b" in Danish...)? Maybe @K-Beicher knows?

I also assumed "v" means "visit". Especially because in the data the event for _v6 fields is besg_6_arm_1, for _v10 besg_10_arm_1, etc. (Does this also mean that the week info is already implicit in the event field?)

Yes, I think you're right and it seems that week is technically redundant with the visit number already being encoded in the event value. But maybe still a friendlier way of showing it than extracting it from the event and mapping the visit to the corresponding week?

joelostblom · 2026-05-27T10:26:44Z

+def _normalise_sefnc_field_name(field_name: str) -> str:
+    return SEFNC_WEEK_FIELD_PATTERN.sub("", field_name).replace(
+        "sefnc_ubusy_schedule", "sefnc_busy_schedule"
+    )


Some of the fields used the name sefnc_ubusy_schedule and some left out the u and called it sefnc_busy_schedule. I don't know if the u is a valuable part to keep or just a type that is fine to remove as I did here.

@K-Beicher?

joelostblom · 2026-05-27T10:31:08Z

            "name": "sefnc_when_busy",
            "title": "sefnc_when_busy",
            "type": "string",
-            "description": "Participant's confidence in following their diet when they have a heavy workload. Self-reported by participant. Repeated at baseline (V4), week 12 (V6) and week 52 (V10).",


This line mentions that the v suffix does not represent the week.

martonvago · 2026-05-28T10:45:12Z

+    "sefnc_baseline_v4": 0,
+    "sefnc_week12_v6": 12,
+    "selfefficacy_for_nutrition_change_sefnc_week_52": 52,
+}


I also assumed "v" means "visit". Especially because in the data the event for _v6 fields is besg_6_arm_1, for _v10 besg_10_arm_1, etc. (Does this also mean that the week info is already implicit in the event field?)

martonvago · 2026-05-28T11:01:06Z

 ) -> list[sp.ResourceProperties]:
    """Converts REDCap data dictionary to Data Package resources."""
-    redcap_fields = _join_vas_time_resources(redcap_fields)
+    redcap_fields = _join_sefnc_week_resources(_join_vas_time_resources(redcap_fields))


Maybe you're already thinking about this in #91, but maybe it would be nicer to have these steps sequential instead of nested. That would allow us to add or remove these transformations in a more modular way.

Exactly. These are the times I wish Python had a pipe 😢

Suggested change

redcap_fields = _join_sefnc_week_resources(_join_vas_time_resources(redcap_fields))

redcap_fields = _join_sefnc_week_resources(redcap_fields)

redcap_fields = _join_vas_time_resources(redcap_fields)

Yup, I agree it's nicer not to nest.

A pipe would be nice too. If we really want something like that, we could consider taking on one of the functional python libraries as a dependency maybe https://toolz.readthedocs.io/en/latest/api.html#toolz.functoolz.pipe (would maybe give more the other functional capabilities as well, but also seems a bit unnecessary if we are likely planning to switch to Rust eventually)?

martonvago · 2026-05-28T13:09:39Z

+    }
+
+
+def _normalise_sefnc_field_name(field_name: str) -> str:


Should we also drop the sefnc_ prefix like with vas_?

Yes! ☺️

lwjohnst86

Nice, just some comments/changes :)

lwjohnst86 · 2026-05-28T13:28:24Z

 ) -> list[sp.ResourceProperties]:
    """Converts REDCap data dictionary to Data Package resources."""
-    redcap_fields = _join_vas_time_resources(redcap_fields)
+    redcap_fields = _join_sefnc_week_resources(_join_vas_time_resources(redcap_fields))


Exactly. These are the times I wish Python had a pipe 😢

Suggested change

redcap_fields = _join_sefnc_week_resources(_join_vas_time_resources(redcap_fields))

redcap_fields = _join_sefnc_week_resources(redcap_fields)

redcap_fields = _join_vas_time_resources(redcap_fields)

lwjohnst86 · 2026-05-28T13:29:21Z

+    }
+
+
+def _normalise_sefnc_field_name(field_name: str) -> str:


Yes! ☺️

lwjohnst86 · 2026-05-28T13:29:35Z

+def _normalise_sefnc_field_name(field_name: str) -> str:
+    return SEFNC_WEEK_FIELD_PATTERN.sub("", field_name).replace(
+        "sefnc_ubusy_schedule", "sefnc_busy_schedule"
+    )


@K-Beicher?

lwjohnst86 · 2026-05-28T13:30:15Z

    ).strip()


+def _join_sefnc_week_resources(


We may have to consider a better way of organizing the scripts, this will start getting big...

Yes I opened #91 for discussing that.

lwjohnst86 · 2026-05-28T13:31:08Z

+    return deduplicated_fields
+
+
+def _append_if_new_sefnc_field(


This could probably be generalised in some way no?

I opened #91 for discussing generalization since I thought it was bigger than just this PR and I wanted to keep the PR small.

lwjohnst86 · 2026-05-28T13:32:02Z

+    if form_name == "sefnc":
+        week_field = sp.FieldProperties(
+            name="week",
+            title="Week",


Would this be week from the first visit?

Hmm I don't think so, it seems like the v4 suffix in the column name was just to represent the "baseline", which I interpreted as week 0 and visit 4. See https://github.com/onlimit-study/feasibility-data/pull/90/changes#r3310224431 for what I based this on.

But I'm not certain, maybe @K-Beicher can clarify further?

lwjohnst86 · 2026-05-28T13:33:19Z

            "title": "sefnc_when_tired",
            "type": "string",
-            "description": "Participant's confidence in following their diet when feeling tired. Self-reported by participant. Baseline (V4).",
+            "description": "Participant's confidence in following their diet when feeling tired. Self-reported by participant.",


"Self-reported by participant" Could probably be removed, as that information is better placed in the resource description.

joelostblom added 2 commits May 27, 2026 12:17

feat: ✨ Join SEFNC week resources

f451a23

Normalize the SEFNC visit-specific REDCap forms into one resource schema with a week field so repeated measurements are represented by rows instead of suffixed columns.

build: 📦 Regenerate SEFNC metadata

401527c

Update the generated data package metadata so SEFNC has one resource with week values 0, 12, and 52 and no visit-suffixed duplicate fields.

joelostblom requested a review from a team as a code owner May 27, 2026 10:23

add-to-board-token Bot added this to Data development May 27, 2026

github-actions Bot assigned joelostblom May 27, 2026

github-project-automation Bot moved this to Todo in Data development May 27, 2026

joelostblom changed the base branch from main to feat/time-suffixes May 27, 2026 10:24

joelostblom commented May 27, 2026

View reviewed changes

joelostblom mentioned this pull request May 27, 2026

Possibly generalize some of the code for dealing with repeated resources? #91

Open

joelostblom moved this from Todo to In review in Data development May 27, 2026

This was referenced May 27, 2026

refactor: ♻️ turn foer_besoegsdag into a single resource #92

Closed

refactor: ♻️ turn foer_besoegsdag into a single resource #93

Open

martonvago reviewed May 28, 2026

View reviewed changes

lwjohnst86 requested changes May 28, 2026

View reviewed changes

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

joelostblom added 3 commits May 28, 2026 19:54

refactor: ♻️ Sequence metadata transformations

eb5c7b0

feat: ✨ Simplify SEFNC field names

9c0dd33

docs: 📝 Consolidate SEFNC self-report context

bb3ff4c

joelostblom requested a review from martonvago May 29, 2026 12:46

joelostblom moved this from In progress to In review in Data development May 29, 2026

joelostblom requested a review from lwjohnst86 May 29, 2026 12:46

	redcap_fields = _join_sefnc_week_resources(_join_vas_time_resources(redcap_fields))
	redcap_fields = _join_sefnc_week_resources(redcap_fields)
	redcap_fields = _join_vas_time_resources(redcap_fields)

Conversation

joelostblom commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joelostblom commented May 27, 2026 •

edited

Loading