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

Example data #27

Merged
merged 4 commits into from
Jan 28, 2022
Merged

Example data #27

merged 4 commits into from
Jan 28, 2022

Conversation

surchs
Copy link
Contributor

@surchs surchs commented Jan 27, 2022

Added some examples for participants.tsv data and (most of) the corresponding json sidecar files. All of the files are real data from openneuro (hence the ds dataset IDs).

(mostly) closes #10

@surchs surchs requested a review from jarmoza January 27, 2022 18:18
@surchs
Copy link
Contributor Author

surchs commented Jan 27, 2022

@jarmoza: let me know where you were thinking these files should go.

@jarmoza
Copy link
Contributor

jarmoza commented Jan 27, 2022

@surchs I think these files should go in the 'assets' folder at root. Maybe put them in their own 'example' subfolder? The assets folder does not yet exist in the main branch.

One issue that needs to be addressed is that the sidecare json files from 431aee0 all just contain 404 messages.

@surchs
Copy link
Contributor Author

surchs commented Jan 27, 2022

@jarmoza: thanks for the points.

I think these files should go in the 'assets' folder at root

I think that's where this PR would put them. Let me know if you think a different place is better

One issue that needs to be addressed is that the sidecare json files from 431aee0 all just contain 404 messages.

Yes, I noticed this too. Not quite sure why, at least some of these files should exist. Will check my downloading code.

@surchs
Copy link
Contributor Author

surchs commented Jan 28, 2022

I think two things were happening here:

  1. I ran a curl script to get participant.json files for every participant.tsv file. I didn't check whether these files actually exist so for the ones that don't, we got the 404
  2. Some of the participant.json files that returned 404 actually exist at the requested address. I haven't really found out why they failed to download the first time (and why some were downloaded successfully the first time). Rerunning this got me the remaining two that exist.

So I removed the participant.json files that don't exist, and added the ones that do. I think a lot of "real world" BIDS datasets come without the participants.json data dictionary. I am not sure that we should support these cases because it'll probably make our lives harder. But I'll leave them in for now since we also have examples with data dictionaries

@jarmoza
Copy link
Contributor

jarmoza commented Jan 28, 2022

@surchs Looks good now. As for whether or not we want to support datasets without a participants.json file, we should decide that. For now, I already have logic built into my incoming PR that allows for just using a participants.tsv file.

Copy link
Contributor

@jarmoza jarmoza left a comment

Choose a reason for hiding this comment

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

Still getting used to the reviewing system. All looks good!

@surchs surchs merged commit e312f45 into master Jan 28, 2022
@surchs surchs deleted the example_data branch January 28, 2022 15:39
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.

Create examples of raw and annotated tabular data
2 participants