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

[MRG] _return_root_paths now only looks in subfolders of root starting with 'sub-' #1253

Closed
wants to merge 7 commits into from

Conversation

kaare-mikkelsen
Copy link
Contributor

PR Description

In relation to this issue: #1127

I now submit this pull request. It's a pretty simple solution - after calling root.rglob(pattern), returned paths are filtered such that only those starting with root / 'sub-' are sent on.

This closes #1127

Merge checklist

Maintainer, please confirm the following before merging.
If applicable:

  • All comments are resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • New contributors have been added to CITATION.cff
  • PR description includes phrase "closes <#issue-number>"

Copy link

welcome bot commented May 24, 2024

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

@kaare-mikkelsen
Copy link
Contributor Author

so, I can see that my commit fails a lot of tests, despite being a very minor change? besides changing the length of line 2444 to be compatible with the style guide, what else should I do?

@sappelhoff
Copy link
Member

@kaare-mikkelsen there were some unrelated failures that should now be fixed in main. I have merged these changes into your branch -- let's see how the CI comes back.

@sappelhoff sappelhoff added this to the 0.16 milestone May 28, 2024
Copy link

codecov bot commented May 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.45%. Comparing base (87eea28) to head (a92016a).
Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1253      +/-   ##
==========================================
- Coverage   97.61%   97.45%   -0.17%     
==========================================
  Files          40       40              
  Lines        8685     8687       +2     
==========================================
- Hits         8478     8466      -12     
- Misses        207      221      +14     

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

@hoechenberger
Copy link
Member

@sappelhoff We should probably set up the autofix.ci bot to make contributors' lives easier

mne_bids/path.py Outdated Show resolved Hide resolved
kaare-mikkelsen and others added 2 commits May 28, 2024 16:24
Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
@kaare-mikkelsen kaare-mikkelsen changed the title _return_root_paths now only looks in subfolders of root starting with 'sub-' [MRG] _return_root_paths now only looks in subfolders of root starting with 'sub-' May 30, 2024
@sappelhoff
Copy link
Member

@kaare-mikkelsen I plan to have a look at this within the next three weeks -- thanks for your patience!

@sappelhoff
Copy link
Member

please see #1281, which replaces this PR.

@sappelhoff sappelhoff closed this Jul 28, 2024
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.

restrict find_matching_paths to only look outside of derivatives/
3 participants