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

Add 'exclude' field #35

Merged
merged 15 commits into from
Jun 22, 2024
Merged

Add 'exclude' field #35

merged 15 commits into from
Jun 22, 2024

Conversation

e-kenneally
Copy link
Contributor

Fixes issue #20

I added exclude flag for the command line which just feeds into the pre-existing 'skip' field at runtime. Tested locally.

Copy link
Collaborator

@clane9 clane9 left a comment

Choose a reason for hiding this comment

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

Thanks @e-kenneally! Left a few minor comments. Can you also add a test_bids2table_exclude in our tests?

bids2table/_b2t.py Outdated Show resolved Hide resolved
bids2table/__main__.py Outdated Show resolved Hide resolved
source = Crawler(
root=root,
include=["sub-*"], # find subject dirs
skip=["sub-*"], # but don't crawl into subject dirs
skip=["sub-*"] + exclude, # but don't crawl into subject dirs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably we should also pass exclude to the Crawler's own exclude arg? But this should only matter in the odd case that someone tries to exclude a particular subject directory (which to be fair I'm not sure why anyone would do).

Copy link
Collaborator

@clane9 clane9 left a comment

Choose a reason for hiding this comment

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

Looks good, good fix to add exclusions to the second-level extract_bids_subdir. (For reference, indexing in two phases like this is easier to parallelize.)

Left a few minor suggestions. Once you run the formatting should be good to go.

bids2table/__main__.py Outdated Show resolved Hide resolved
bids2table/_b2t.py Outdated Show resolved Hide resolved
bids2table/extractors/bids.py Outdated Show resolved Hide resolved
@clane9 clane9 merged commit abeff96 into main Jun 22, 2024
3 checks passed
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.

2 participants