Skip to content

FIX: Filtering in BIDSLayoutIndexer#1063

Open
alperkent wants to merge 3 commits into
bids-standard:mainfrom
alperkent:master
Open

FIX: Filtering in BIDSLayoutIndexer#1063
alperkent wants to merge 3 commits into
bids-standard:mainfrom
alperkent:master

Conversation

@alperkent

@alperkent alperkent commented May 10, 2024

Copy link
Copy Markdown
Contributor

Resolves (partially) #1062.

Remove if clause on self.filters
@alperkent alperkent reopened this May 22, 2024
@codecov

codecov Bot commented May 22, 2024

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.74%. Comparing base (20007e5) to head (9b9c74e).
⚠️ Report is 344 commits behind head on main.

Files with missing lines Patch % Lines
bids/layout/index.py 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
- Coverage   89.80%   89.74%   -0.06%     
==========================================
  Files          63       63              
  Lines        7177     7179       +2     
  Branches     1374     1375       +1     
==========================================
- Hits         6445     6443       -2     
- Misses        532      535       +3     
- Partials      200      201       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@effigies effigies left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm realizing that the **filters argument only says it applies to filtering files opened during get_metadata(). It was not intended to filter files.

That said, I can't think of a good reason to filter metadata but not files. @adelavega Do you remember the reasoning here?

Comment thread bids/layout/index.py
Comment on lines +253 to 256
# Skip entities that don't match the filter
if self.filters and e.name and e.name in self.filters and m and m not in self.filters[e.name]:
continue
if m is not None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the equivalent of

Suggested change
# Skip entities that don't match the filter
if self.filters and e.name and e.name in self.filters and m and m not in self.filters[e.name]:
continue
if m is not None:
if m is not None and (e.name not in self.filters or m in listify(self.filters[e.name])):

@adelavega

Copy link
Copy Markdown
Collaborator

I'm not sure tbh.

I traced this back to; #560

Then Yarkoni moved it around.

Looks like we thought at the time that meta-data indexing was the most time consuming step, and implemented a way to skip meta-data indexing for a subset of files, rather then excluding them from being indexed altogether.

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.

3 participants