Skip to content

chore: 🍱 split REDCap data into one Parquet file per form - #192

Open
martonvago wants to merge 22 commits into
mainfrom
chore/split-forms
Open

chore: 🍱 split REDCap data into one Parquet file per form#192
martonvago wants to merge 22 commits into
mainfrom
chore/split-forms

Conversation

@martonvago

@martonvago martonvago commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR splits REDCap data into one Parquet file per form. I am saying "form" intentionally because moving to resources will only come in the next step. Some forms correspond one-to-one to resources, but some are first transformed or joined (e.g. vas). For the first kind, we can just move the parquet files generated here straight over to staging. For the second kind, we will do the necessary transformations when staging.

Of interest is the handling of metadata fields/columns that are not present in some data batches. This happens when new columns are added, which are missing in old batches. To make all staging data have the same shape, I add the missing columns in this step (filled with null). This should work out nicely because, when we join staging batches, newer batches will trump older batches. So values in the new columns in newer batches will trump the null placeholders.

For now, this only deals with participant_id, event_id, and submission_id. study_week_id and potentially other ids will fit roughly where event_id goes. There's already plenty in this PR to discuss, so I think it's simpler to let the study_week_id idea mature and refactor later. Structurally, (participant_id, event_id, submission_id) works as a PK, so we can continue building the pipeline in the meantime.

I tested this on GenomeDK and it ran in 50ish seconds, so that's rather slow. Room for optimisation... down to 5 with some magic and sorcery 🧙

Closes #169

Needs a thorough review.

Checklist

  • Ran just run-all

@martonvago martonvago self-assigned this Aug 5, 2026
@martonvago martonvago moved this from Todo to In Progress in Data development Aug 5, 2026
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.

Comment on lines +93 to +101
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)
)

@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.


@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.


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?

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)
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.

return _with_missing_columns(raw_lf, form_to_fields)


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?


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.)

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.

),
]

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.

@martonvago martonvago moved this from In Progress to In Review in Data development Aug 6, 2026
@martonvago
martonvago marked this pull request as ready for review August 6, 2026 13:22
@martonvago
martonvago requested a review from a team as a code owner August 6, 2026 13:22
@lwjohnst86 lwjohnst86 changed the title chore: 🍱 split REDCap data into one Parquet file per form chore: 🍱 split REDCap data into one Parquet file per form Aug 6, 2026

@lwjohnst86 lwjohnst86 left a comment

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.

Nice start 🎉 Here's some initial comments and suggestions, haven't fully reviewed every bit.


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
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)


def get_form_field_mapping(

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.

Comment on lines +66 to +71
forms = so.fmap(
form_to_fields.items(),
lambda form_entry: _create_df_for_form(
form_entry, raw_lf, form_to_events, repeating_form_names
),
)

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 Form(name=form_name, data=raw_lf.filter(filters).select(columns).collect())

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.

Comment on lines +102 to +112
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)

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"?

@github-project-automation github-project-automation Bot moved this from In Review to In Progress in Data development Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Split raw data into into one Parquet file per form

2 participants