Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix nan #81

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

fix nan #81

wants to merge 12 commits into from

Conversation

yibeichan
Copy link
Contributor

regarding this issue
sensein/b2ai-redcap2rs#5

@Evan8456
Copy link
Contributor

Looks good!

@@ -420,7 +420,15 @@ def process_row(
# )

elif key in ADDITIONAL_NOTES_LIST and value:
notes_obj = {"source": "redcap", "column": key, "value": value}
# Convert value to string, handling NaN explicitly
value_str = "NaN" if pd.isna(value) else str(value).strip()
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really want to do this or skip nans?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question. so in the b2ai case, what happens when we have "value": NaN? @ibevers what do you mean by "Items that have NaN in them don't work in reproschema-ui." they won't appear in the UI or they can't proceed to the next one?

Copy link

Choose a reason for hiding this comment

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

You can't proceed @yibeichan

@satra
Copy link
Contributor

satra commented Jan 17, 2025

i left some suggestions, but we also need to check for nan in other contexts.

@yibeichan
Copy link
Contributor Author

we made changes to handle "nan" when converting from redcap to reproschema; however, we are failing test_rs2redcap_redcap2rs which use nimh-minimal to compare two directional conversion. i think the error is related to "remove the field when value is nan"
we need a separate PR to address this issue in reproschema2redcap.py, we can merge the current PR first
I made a lot of changes in this one tho...
(also I found a matrixinfo related issue, probably needs another PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants