feat: ✨ convert redcap data dict to resource properties#35
feat: ✨ convert redcap data dict to resource properties#35martonvago wants to merge 12 commits intomainfrom
Conversation
| def _map(x: Iterable[In], fn: Callable[[In], Out]) -> list[Out]: | ||
| return list(map(fn, x)) | ||
|
|
||
|
|
||
| def _filter(x: Iterable[In], fn: Callable[[In], bool]) -> list[In]: | ||
| return list(filter(fn, x)) | ||
|
|
||
|
|
||
| def _flat_map(items: Iterable[In], fn: Callable[[In], Iterable[Out]]) -> list[Out]: | ||
| """Maps and flattens the items by one level.""" | ||
| return list(chain.from_iterable(map(fn, items))) |
There was a problem hiding this comment.
These could come from soil.
Or we could decide #34 and put them in internals.py
There was a problem hiding this comment.
Yea, agreed, we use them in almost all our packages/code, so makes sense to move them into their own package 👍
| form_name: str, fields: list[dict[str, str]] | ||
| ) -> sp.ResourceProperties: | ||
| visit_field = sp.FieldProperties( | ||
| name="visit", |
There was a problem hiding this comment.
Or event?
Could contain the unique_event_name or event_id of a REDCap event
There was a problem hiding this comment.
Hmm, I don't have a strong opinion on this. I think visit is fine, @K-Beicher thoughts?
There was a problem hiding this comment.
Not all events will be visits, and event is a well established REDCap concept, that would lean towards using either event as the word (if I understand your discussion correctly).
| title=form_name, | ||
| description=form_name, |
There was a problem hiding this comment.
The title could be the instrument_label that we can get by making another API call (it's not in the data dict).
I wasn't able to find an equivalent for description.
| title=field["field_name"], | ||
| type=_get_type(field), | ||
| description=_get_description(field), | ||
| categories=_get_categories(field), |
There was a problem hiding this comment.
This is a list of strings.
We can also have a list of objects like {"value": 1, "label": "apple"} to keep track of the REDCap number of the choices as well.
There was a problem hiding this comment.
Yea, we might have to organise the scripts for both the properties and for tidying up the data.
lwjohnst86
left a comment
There was a problem hiding this comment.
Nice start! It would be better to actually see what the properties output is though.
| def _map(x: Iterable[In], fn: Callable[[In], Out]) -> list[Out]: | ||
| return list(map(fn, x)) | ||
|
|
||
|
|
||
| def _filter(x: Iterable[In], fn: Callable[[In], bool]) -> list[In]: | ||
| return list(filter(fn, x)) | ||
|
|
||
|
|
||
| def _flat_map(items: Iterable[In], fn: Callable[[In], Iterable[Out]]) -> list[Out]: | ||
| """Maps and flattens the items by one level.""" | ||
| return list(chain.from_iterable(map(fn, items))) |
There was a problem hiding this comment.
Yea, agreed, we use them in almost all our packages/code, so makes sense to move them into their own package 👍
| form_name: str, fields: list[dict[str, str]] | ||
| ) -> sp.ResourceProperties: | ||
| visit_field = sp.FieldProperties( | ||
| name="visit", |
There was a problem hiding this comment.
Hmm, I don't have a strong opinion on this. I think visit is fine, @K-Beicher thoughts?
There was a problem hiding this comment.
I think it would help to see the output of this with the actual metadata converted to properties. Can you add that here too?
There was a problem hiding this comment.
Also, the file name could be shortened to something like redcap_dict_to_properties.py. As we get more files, we'll have to think what the best naming scheme is.
There was a problem hiding this comment.
A lot of this code could potentially be moved into a "shears" package or other toolkit type name, since the REDCap dictionary to properties conversion could potentially be a very common thing to do.
| return list(chain.from_iterable(map(fn, items))) | ||
|
|
||
|
|
||
| def load_data_dict_from_file() -> list[dict[str, str]]: |
There was a problem hiding this comment.
Could be renamed to read_dictionary()
| return json.load(f) | ||
|
|
||
|
|
||
| def redcap_data_dict_to_resource_properties( |
There was a problem hiding this comment.
| def redcap_data_dict_to_resource_properties( | |
| def dictionary_to_properties( |
Just to simplify the naming. It's in the redcap script, so don't really need to use redcap in the name.
| ) | ||
|
|
||
|
|
||
| def _redcap_form_to_resource( |
There was a problem hiding this comment.
Same comment as above about redcap
| title=field["field_name"], | ||
| type=_get_type(field), | ||
| description=_get_description(field), | ||
| categories=_get_categories(field), |
There was a problem hiding this comment.
Yea, we might have to organise the scripts for both the properties and for tidying up the data.
| constraints=sp.ConstraintsProperties(required=True), | ||
| ) | ||
| center_field = sp.FieldProperties( | ||
| name="center", |
There was a problem hiding this comment.
Or centre 🇬🇧 :P
| min_length=_get_text_length_bound(field, "text_validation_min"), | ||
| max_length=_get_text_length_bound(field, "text_validation_max"), |
There was a problem hiding this comment.
I haven't actually seen these in the wild
|
I added the generated resource properties to the package properties and updated |
Description
This PR adds the ability to convert a saved REDCap data dict to resource properties.
Followed this plan: #23 (comment)
Does not include a resource for events/visits. Not sure if we want that?
Closes #25 closes #26
This PR needs an in-depth review.
Checklist
just run-all