allow missing sessions

[drammock]

WIP. Closes #992

Before merging …

• Changelog has been updated (docs/source/changes.md)

[larsoner]

Nice, I was hoping it would look about this simple! Thanks @drammock

[larsoner]

... just noticed you had WIP in the title. Let me know when I should look. In the meantime the CIs are giving it a shot but I wouldn't be surprised if they failed because of mne-tools/mne-bids#1311

[drammock]

OK, I need help. I don't really understand what is going on in _docs.py (more/better comments would help!) so I can't figure out what to change to get the test_config_options_used to pass. Tried:

• adding allow_missing_sessions to _FORCE_EMPTY
• adding _config_utils.get_subjects_sessions to the helper functions early in the __init__ of _ParseConfigSteps
• both at once

[larsoner]

Naively I would expect you to be able just to add it to the one place in _docs.py where you find get_subjects (and get_sessions) for example but it's probably more than that. I'd have to look. BTW you can manually run different doc-building .py files to replicate the error and very quickly iterate rather than doing a quick doc build.

Do you want me to take a look?

[drammock]

Naively I would expect you to be able just to add it to the one place in _docs.py where you find get_subjects (and get_sessions) for example but it's probably more than that.

nope. get_subjects_sessions only ever appears in main() functions, and get_subjects / get_sessions only appear in _docs later on when you're handling get_config funcs.

BTW you can manually run different doc-building .py files to replicate the error and very quickly iterate rather than doing a quick doc build.

Not following what you mean here --- the failure in question is a pytest test, not doc build?

Do you want me to take a look?

I've pushed 2 commits, one with stuff that I thought would work (or at least be needed) but wasn't after all (once I figured out the hack in the second commit). Please view the diff of those two commits separately, then comment on how I can make it less hacky. The 3rd commit (about [None]) is unrelated, and should fix other test failures.

[larsoner]

The AST parsing I wrote wasn't good/complete enough. Needed to iterate over call.args in addition to call.kwargs. Pushed a commit to fix it.

[drammock]

The AST parsing I wrote wasn't good/complete enough. Needed to iterate over call.args in addition to call.kwargs. Pushed a commit to fix it.

OK thanks! WDYT about testing? It looks like ds001810 has one subj w/ multiple sessions so it should work as a test case. Thinking maybe use tmpdir as BIDSRoot, and in it create links to just one session from that subj, but have config list 2 sessions. If you think there's a better/easier way LMK.

After that, I'll add a changelog then, and I think we'll be good!

[larsoner]

Thinking maybe use tmpdir as BIDSRoot, and in it create links to just one session from that subj, but have config list 2 sessions. If you think there's a better/easier way LMK.

Yes that could work! You don't even need to run any steps from the pipeline beyond the init_derivatives looks like so it could be very quick. You could run it first without the setting set, ensure it raises an error about a missing session, then modify the temp config file to be permissive, and ensure the init step runs.

[larsoner]

... FWIW so far I don't think we have many (any?) tests like this so you might be starting from scratch a bit. But it will be nice to have some code for more targeted testing. So far if we add a new feature or behavior we have tried to make sure we could modify some EDIT: full dataset run to make use of it, which isn't really sustainable as the number of options and exceptions grows.

[drammock]

OK @larsoner assuming all CIs are still green, this one should be ready!

[larsoner]

Ahh that CI does not have ds001810 downloaded. Might be better to mock a little dataset rather than assume it's available. More work but will give us more flexibility down the line. I can give it a shot if you want

[drammock]

Ahh that CI does not have ds001810 downloaded. Might be better to mock a little dataset rather than assume it's available. More work but will give us more flexibility down the line. I can give it a shot if you want

wait, which one? The only CI failure I see now (check docs consistency) is that Path.walk() only exists since Py 3.12. Can we bump that CI to 3.12 or should I swap in os.walk etc?

[larsoner]

Oh if it's a 3.12 thing then yes feel free just to bump the version there

[drammock]

Ahh that CI does not have ds001810 downloaded. Might be better to mock a little dataset rather than assume it's available. More work but will give us more flexibility down the line. I can give it a shot if you want

wait, which one? The only CI failure I see now (check docs consistency) is that Path.walk() only exists since Py 3.12. Can we bump that CI to 3.12 or should I swap in os.walk etc?

ah OK, now it's showing up again :(

 E   FileNotFoundError: The following requested subjects were not found in the dataset: 01

I'll mock a dataset

[drammock]

all green!

[hoechenberger]

otherwise, seems good to me! :) nicely done, @drammock

@larsoner I'll leave the final review / decision to you

[larsoner]

Thanks @drammock !