Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c865a74
chore: 🏗️ add event and repeating forms metadata download to pytask
martonvago Jul 24, 2026
e34ba7e
chore: 🔧 update lockfile
martonvago Jul 24, 2026
23b4c6a
Merge branch 'main' into chore/pytask-other-metadata
martonvago Jul 29, 2026
ccff78c
Merge branch 'main' into chore/pytask-other-metadata
martonvago Jul 30, 2026
9e665a5
Merge branch 'main' into chore/pytask-other-metadata
martonvago Jul 31, 2026
3b0effb
Merge branch 'main' into chore/pytask-other-metadata
martonvago Aug 3, 2026
8928a40
Merge branch 'chore/pytask-other-metadata' into chore/split-forms
martonvago Aug 3, 2026
cc620fa
chore: :bento: split forms
martonvago Aug 4, 2026
8c73cef
refactor: ♻️ small tweaks
martonvago Aug 5, 2026
6482294
chore: 🔧 fix imports after merge
martonvago Aug 5, 2026
e82797a
Merge branch 'chore/fix-imports' into chore/split-forms
martonvago Aug 5, 2026
86e0332
fix: 🐛 handle case when metadata has more columns than data
martonvago Aug 5, 2026
730aed5
Merge branch 'main' into chore/split-forms
martonvago Aug 5, 2026
908bfd0
perf: ⚡️ use lazy frames
martonvago Aug 6, 2026
a4f28e5
refactor: ♻️ use small dataclass instead of extra column to track for…
martonvago Aug 6, 2026
14ee138
refactor: ♻️ move metadata processing out of loop
martonvago Aug 6, 2026
8616731
refactor: ♻️ move write call to build.py
martonvago Aug 6, 2026
5fa8a88
refactor: ♻️ move read call to build.py
martonvago Aug 6, 2026
8684da1
refactor: ♻️ move column addition into read
martonvago Aug 6, 2026
202e43f
chore: 🔧 update lockfile
martonvago Aug 6, 2026
6987d59
refactor: ♻️ move timestamp extraction to core
martonvago Aug 6, 2026
4bd0c4a
docs: 📝 tweak docstring
martonvago Aug 6, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 40 additions & 5 deletions src/feasibility_data/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@

BLD_REDCAP = BLD / "redcap"
RAW_REDCAP = RAW / "redcap"
RAW_REDCAP_DATA = DirectoryNode(root_dir=RAW_REDCAP, pattern="*.csv.gz")

RAW_MYFOOD24 = RAW / "myfood24"

FIELD_METADATA_PATH = BLD_REDCAP / "field_metadata.json"
EVENT_METADATA_PATH = BLD_REDCAP / "event_metadata.json"
REPEATING_FORMS_METADATA_PATH = BLD_REDCAP / "repeating_forms_metadata.json"
FIELD_METADATA_PREPROCESSED_PATH = BLD_REDCAP / "field_metadata_preprocessed.json"
FORMS = BLD_REDCAP / "forms"


def task_download_field_metadata(
Expand Down Expand Up @@ -50,11 +52,7 @@ def task_download_repeating_forms_metadata(


def task_download_raw_redcap_data(
raw_data_dir: Annotated[
Path,
DirectoryNode(root_dir=RAW_REDCAP, pattern="*.csv.gz"),
Product,
],
raw_data_dir: Annotated[Path, RAW_REDCAP_DATA, Product],
) -> None:
"""Download the latest data from all centers to `RAW_REDCAP/<timestamp>.csv.gz`."""
# TODO: Handle all centers
Expand All @@ -77,6 +75,43 @@ def task_preprocess_field_metadata(
common.json.write(field_metadata_preprocessed_path, field_metadata_preprocessed)


def task_split_forms(

@martonvago martonvago Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I moved some complexity into this function as we wanted these tasks to do orchestration rather than just call another function that does the orchestration.

I do all reading and writing here to match previous tasks.

forms_dir: Annotated[
Path,
DirectoryNode(root_dir=FORMS, pattern="**/*.parquet"),
Product,
],
raw_data_paths: Annotated[list[Path], RAW_REDCAP_DATA],
field_metadata_path: Path = FIELD_METADATA_PREPROCESSED_PATH,
event_metadata_path: Path = EVENT_METADATA_PATH,
repeating_forms_path: Path = REPEATING_FORMS_METADATA_PATH,
) -> None:
"""Split each batch of raw data into one Parquet file per form.

Written to `FORMS/<timestamp>/<form_name>.parquet`.
"""
form_to_fields = data.redcap.core.get_form_field_mapping(
common.json.read(field_metadata_path)
)
form_to_events = data.redcap.core.get_form_event_mapping(
common.json.read(event_metadata_path)
)
repeating_form_names = data.redcap.core.get_repeating_forms(
common.json.read(repeating_forms_path)
)
Comment on lines +93 to +101

@martonvago martonvago Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is the metadata we will use to split the data.
I could unite them into a single structure if people like that better, e.g.:

[
  "bedq": {
    "fields": [...],
    "events": [...],
    "repeats": False,
  },
  ...
]

Could even be the output of the previous preprocessing step.

for raw_data_path in raw_data_paths:
raw_data = data.redcap.core.read_raw(raw_data_path, form_to_fields)
forms = data.redcap.core.split_forms(
raw_data,
form_to_fields,
form_to_events,
repeating_form_names,
)

for form in forms:
data.redcap.core.write_form(form, forms_dir, raw_data_path)
Comment on lines +102 to +112

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.

This can be rewritten with map, which might allow us to use some parallel processing from the refactor, which will help speed things up.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There is a paralleliser plugin for Pytask, which could fit nicely here to parallelise the iterations of the outer loop (i.e. processing the raw batches). This would be equivalent to creating a map and executing in parallel.

Or do you mean something more specific by "the refactor"?



def task_download_myfood24_data(
myfood24_raw_data_dir: Annotated[
Path,
Expand Down
4 changes: 2 additions & 2 deletions src/feasibility_data/data/redcap/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""REDCap data functions."""

from . import raw
from . import core, raw

__all__ = ["raw"]
__all__ = ["core", "raw"]
128 changes: 128 additions & 0 deletions src/feasibility_data/data/redcap/core.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
from collections import defaultdict
from dataclasses import dataclass
from operator import itemgetter
from pathlib import Path

import polars as pl
import seedcase_soil as so


@dataclass
class Form:
"""Class to hold the name and data of a form."""

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I decided to do this instead of writing the form name into each df as a separate column only to drop that column later, as it felt a bit cleaner.


name: str
data: pl.DataFrame


REDCAP_ID_COLS = [
"record_id_s",
"redcap_event_name",
"redcap_repeat_instrument",
"redcap_repeat_instance",
]


def read_raw(raw_data_path: Path, form_to_fields: dict[str, list[str]]) -> pl.LazyFrame:
"""Read the raw data into a LazyFrame with missing columns added."""
raw_lf = pl.scan_csv(raw_data_path, infer_schema=False)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm reading into a lazy frame because the huge number of columns made the transformations very slow on data frames, even with only 2 raw batches. Lazy frames allow Polars to optimise operations a lot more.

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.

This should be added as a comment, since it helps communicate why LazyFrame was used. Can you add that as a comment?

return _with_missing_columns(raw_lf, form_to_fields)

@martonvago martonvago Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I decided to add missing columns here, right at read-time. This means that we don't have to worry about this later on in the flow. Can be a separate step of course.

I also thought about extra columns (if a column is dropped later on in the study), but I don't think that's a problem. Any columns not in the latest metadata will not make it into staging, which feels like what we want.

Let me know if anyone has wise thoughts about these scenarios.



def get_form_field_mapping(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

These mapping functions use a for loop. I think this is the simplest and cleanest way of expressing the logic. But lmk if I should rewrite it.

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.

It's hard to follow what's going on here and why these are needed, so it's hard for me to know what or if the loop is doing/how to improve on it. Could you refactor these to take a type/class and output a type/class that represents what you actually want/need from the forms? My instinct tells me this could be simpler but I can't pinpoint how yet.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Would this suggestion help?

field_metadata: list[dict[str, str]],
) -> dict[str, list[str]]:
"""Get a mapping from form name to field names in that form."""
mapping: dict[str, list[str]] = defaultdict(list)
for field in field_metadata:
mapping[field["form_name"]].append(field["field_name"])

return mapping


def get_form_event_mapping(
event_metadata: list[dict[str, str]],
) -> dict[str, list[str]]:
"""Get a mapping from form name to event names where the form is filled in."""
mapping: dict[str, list[str]] = defaultdict(list)
for item in event_metadata:
mapping[item["form"]].append(item["unique_event_name"])

return mapping


def get_repeating_forms(repeating_forms: list[dict[str, str]]) -> set[str]:
"""Get the set of repeating form names."""
return set(so.fmap(repeating_forms, itemgetter("form_name")))


def split_forms(
raw_lf: pl.LazyFrame,
form_to_fields: dict[str, list[str]],
form_to_events: dict[str, list[str]],
repeating_form_names: set[str],
) -> list[Form]:
"""Split the raw data into one dataframe per form."""
forms = so.fmap(
form_to_fields.items(),
lambda form_entry: _create_df_for_form(
form_entry, raw_lf, form_to_events, repeating_form_names
),
)
Comment on lines +66 to +71

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.

I believe that Polars has a way to do this rather than use maps, which will probably save a lot of time. A quick look seems that maybe partition_by() might be it..? I know it's possible to do this in R, so I assume Polars can do it to.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think partition_by is for grouping rows by the values in one or more columns, not grouping columns based on their names. I wasn't able to find a Polars-native way of doing what we want, but if you know what exactly you would use in R, maybe we could check whether that has an equivalent.

return so.keep(forms, lambda form: not form.data.is_empty())


def write_form(form: Form, forms_dir: Path, raw_data_path: Path) -> None:
"""Write the dataframe."""
timestamp = raw_data_path.name.removesuffix(".csv.gz")
file_path = forms_dir / timestamp / f"{form.name}.parquet"
file_path.parent.mkdir(parents=True, exist_ok=True)
form.data.write_parquet(file_path)


def _with_missing_columns(
lf: pl.LazyFrame, form_to_fields: dict[str, list[str]]
) -> pl.LazyFrame:
"""Add any missing metadata fields as columns in the dataframe."""
metadata_fields = so.flat_fmap(form_to_fields.values(), lambda fields: fields)
data_fields = set(lf.collect_schema().names())
missing_fields = so.keep(metadata_fields, lambda field: field not in data_fields)
return lf.with_columns(so.fmap(missing_fields, pl.lit(None).alias))


def _create_df_for_form(
form_entry: tuple[str, list[str]],
raw_lf: pl.LazyFrame,
form_to_events: dict[str, list[str]],
repeating_form_names: set[str],
) -> Form:
form_name, field_names = form_entry
events = form_to_events.get(form_name, [])
is_repeating = form_name in repeating_form_names
content_fields = so.keep(field_names, lambda field: field not in REDCAP_ID_COLS)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@signekb maybeee removing the admin fields could be part of this? We will include only the content_fields for each form, so we could strip out admin fields as well.


columns = [
pl.col("record_id_s").alias("participant_id"),
pl.col("redcap_event_name").alias("event_id"),

@martonvago martonvago Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is where the other ids would be set up. (See PR description.)

*so.fmap(content_fields, pl.col),
# TODO: handle different centers
pl.lit("Copenhagen").alias("center"),
]

if is_repeating:
# Submissions for the same participant at the same event are told apart by
# `redcap_repeat_instance`.
columns.insert(
2, pl.col("redcap_repeat_instance").cast(pl.String).alias("submission_id")
)

filters = [
# Keep only rows coming from events where the form was filled in
pl.col("redcap_event_name").is_in(events),
# Keep only non-empty rows
pl.any_horizontal(
so.fmap(content_fields, lambda field: pl.col(field).is_not_null())
),
]

return Form(name=form_name, data=raw_lf.filter(filters).select(columns).collect())

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is where the data frame is materialised (i.e. the operations are executed).

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.

You might not have to collect at this stage, as writing to file will force it. At least that's how it could be done in R, and I know Polars has lots of similarities. But it might be different.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have to collect before writing to check if the df is empty because we said we didn't want to create empty resources. Buuut we could actually allow empty resources in staging and let the properties extraction and batch joining mechanisms deal with them later. Or there are other alternatives to the is_empty check, such as writing without checking and deleting afterwards if the Parquet metadata says 0 rows.

In any case, we should definitely optimise by collecting all frames at once.

12 changes: 6 additions & 6 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.