search

manybabies / mb1-analysis-public

ManyBabies1 analysis code for public sharing
MIT License
6 stars 15 forks source link

new subids (post-unification) and metadata files using old subid ("pre-unification") names #15

Closed mzettersten closed 5 years ago

mzettersten commented 5 years ago

It looks like subids were (sensibly) recoded to merge lab name and subid, to avoid issues due to different labs using the same subid (which causes problems in the mixed-effects regressions).

I haven't looked at this in depth yet, but I think this will cause issues in the variable validation portion of the scripts, whenever we make exclusions or adjustments based on metadata files that rely on the "old" subid names (i.e. the ones that have not been merged with labid). The metadata files that are used to recode session errors are an example of this (but there may be others?). I don't think this should be too tricky to fix, since the metadata files that I'm aware of should include lab and subid columns. But I just wanted to make sure this is on people's radar @mcfrank @jkosie @melsod.

If I'm diagnosing the issue correctly, I can see two potential solutions:

  1. we could update all of the metadata files (and any other files that might have used the old coding?)
  2. Rather than overwriting the old subid column, we could create a new subid column (e.g., lab_subid or whatever) that is then subsequently used for the by-subject random effects. Downside would be possible confusion for someone coming to the dataset fresh. Upside is that this might require fewer changes to existing files/ processing steps.
jkosie commented 5 years ago

I handled this a little while ago for MB1B by updating metadata files. I just merged the lab and subid columns in Excel and it only took a couple minutes to do all the files. I'm happy to do it for MB1 as well if it seems like the way to go.

mcfrank commented 5 years ago

sorry, I'm aware of this issue and just didn't get a chance to fix the pipeline after I integrated the change (ran out of time last friday). no need to do it manually Jessica!

On Tue, Sep 3, 2019 at 2:16 PM Jessica Kosie notifications@github.com wrote:

I handled this a little while ago for MB1B by updating metadata files. I just merged the lab and subid columns in Excel and it only took a couple minutes to do all the files. I'm happy to do it for MB1 as well if it seems like the way to go.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/manybabies/mb1-analysis-public/issues/15?email_source=notifications&email_token=AAI25FY557F6H4K35Y3UNRTQH3H3BA5CNFSM4ITJQQCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5ZTKXI#issuecomment-527643997, or mute the thread https://github.com/notifications/unsubscribe-auth/AAI25F4PRHDEECYKERUUPLDQH3H3BANCNFSM4ITJQQCA .

mzettersten commented 5 years ago

Great! Let me know if I can be helpful in some way.

mcfrank commented 5 years ago

Ok, we now have subid_unique following martin's suggestion. Thanks!