Expose ignore_json, add ignore_nosub to file match/search funcs

[sappelhoff]

• closes restrict find_matching_paths to only look outside of derivatives/ #1127 by replacing it

@kaare-mikkelsen I have finally had time to look at your PR. I suggest the following changes to it. This is mainly about making the newly proposed funcationality configurable via a parameter that is exposed via the public API.

What do you think?

Any thoughts @hoechenberger?

[sappelhoff]

@kaare-mikkelsen I have also taken the liberty to add you to our authors and citation lists, as well as to create an entry for the changelog. Note that these are steps in our contribution guide, but given that I was so late with reviewing your PR, I felt a bit guilty and added this for you 👼

one more note for your (potential) future contributions (to any GitHub project): You have forked mne-bids under a new name: https://github.com/kaare-mikkelsen/mne-bids_fixing --> this was making it a bit hard for me to work with your original PR. If possible, please fork repositories under their real name, and then develop under a new branch (not main, like you did for your contribution). I hope this feedback helps! :-)

[kaare-mikkelsen]

@kaare-mikkelsen I have also taken the liberty to add you to our authors and citation lists, as well as to create an entry for the changelog. Note that these are steps in our contribution guide, but given that I was so late with reviewing your PR, I felt a bit guilty and added this for you 👼

one more note for your (potential) future contributions (to any GitHub project): You have forked mne-bids under a new name: https://github.com/kaare-mikkelsen/mne-bids_fixing --> this was making it a bit hard for me to work with your original PR. If possible, please fork repositories under their real name, and then develop under a new branch (not main, like you did for your contribution). I hope this feedback helps! :-)

thank you, the feedback does help :)

[kaare-mikkelsen]

• closes restrict find_matching_paths to only look outside of derivatives/ #1127 by replacing it

@kaare-mikkelsen I have finally had time to look at your PR. I suggest the following changes to it. This is mainly about making the newly proposed funcationality configurable via a parameter that is exposed via the public API.

What do you think?

Any thoughts @hoechenberger?

Hi @sappelhoff
As I understand it, the only real change between my PR and your suggestion is that the 'sub-folder-only' has to be specifically turned on. Is that correct? As the original issue (described in #1127) was that the standard functionality actually returned paths for non-existent files, wouldn't it be better for the default behavior to not make an error like that? I understand the reasoning behind making 'the old behavior' default, but I think that is more relevant when we are not actually fixing bugs? So I would suggest making the default value of 'ignore_nosub' 'True'. Then people who need the old functionality still have it, but newcomers won't get paths to non-existent files.

[sappelhoff]

As I understand it, the only real change between my PR and your suggestion is that the 'sub-folder-only' has to be specifically turned on. Is that correct?

Yes, I made your changes configurable and put the default on previous behavior, as you correctly point out.

My reasoning is that the current behavior has been in place for a long time, with only one report (yours) about it being different from expected. I think it is the safe route to keep the behavior as is, and leave the option open to change the default if we see more reports of the nature of the report you gave earlier.

[hoechenberger]

@sappelhoff I won't have any time to look into this for the next couple of days, sorry

please ping me again sometime at the end of the week if you still need my feedback

thanks

[sappelhoff]

please ping me again sometime at the end of the week if you still need my feedback

I' appreciate it, but no worries if you don't find the time for it. I'll basically leave this open for a potential review of yours until next Monday or so and just merge otherwise.

If any other mne devs read this, feel free to chime in, too 🙏

[sappelhoff]

Thanks for your contribution @kaare-mikkelsen!