-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fixed bug in redcap2reproschema with items id not matching item filname and redcap variable… #65
Conversation
Evan8456
commented
Jul 11, 2024
- When running redcap2reproschema, the resulting item id's did not match the filename and the variable name on redcap.
@@ -252,18 +252,9 @@ def process_row( | |||
add_preable=True, | |||
): | |||
"""Process a row of the REDCap data and generate the jsonld file for the item.""" | |||
global matrix_group_count |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we remove the global variable as well? i don't think this is used anywhere (please do check).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matrix_group_count was only referenced in the chunk of code I removed, and doesn't show up anywhere else in the project. I think it is safe to remove it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Evan8456 - there is declaration at the top of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, what I meant to say was that now there is no longer a use for matrix_group_count now so I think it is okay to remove the defalcation in addition to the chunk of code I removed earlier. I added a new commit removing the declaration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@satra - re matrix_group_count
I have not removed the parts related to it, since I understood that we might be using in the near future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is something where we need to see an example of to see what we just removed.