mne-tools / mne-bids-pipeline

Automatically process entire electrophysiological datasets using MNE-Python.
https://mne.tools/mne-bids-pipeline/
BSD 3-Clause "New" or "Revised" License
140 stars 67 forks source link

ENH: Validate inputs with pydantic #779

Closed larsoner closed 1 year ago

larsoner commented 1 year ago

Before merging …

Locally it seems to work -- adding config_validation = "foo" to config_ds003392.py for example produces:

$ pytest mne_bids_pipeline -xk 003392
...
mne_bids_pipeline/_config_import.py:410: in _pydantic_validate
    raise ValueError(str(err)) from None
E   ValueError: 1 validation error for user configuration from /home/larsoner/python/mne-bids-pipeline/mne_bids_pipeline/tests/configs/config_ds003392.py
E   config_validation
E     Input should be 'raise', 'warn' or 'ignore' [type=literal_error, input_value='foo', input_type=str]
E       For further information visit https://errors.pydantic.dev/2.1/v/literal_error

or when setting mf_destination = "foo" in config_ds003392.py which allows a specific Literal or a FloatArrayLike:

$ pytest mne_bids_pipeline -xk 004229
...
mne_bids_pipeline/_config_import.py:410: in _pydantic_validate
    raise ValueError(str(err)) from None
E   ValueError: 2 validation errors for user configuration from /home/larsoner/python/mne-bids-pipeline/mne_bids_pipeline/tests/configs/config_ds004229.py
E   mf_destination.literal['reference_run']
E     Input should be 'reference_run' [type=literal_error, input_value='foo', input_type=str]
E       For further information visit https://errors.pydantic.dev/2.1/v/literal_error
E   mf_destination.function-plain[assert_float_array_like()]
E     Value error, could not convert string to float: 'foo' [type=value_error, input_value='foo', input_type=str]
E       For further information visit https://errors.pydantic.dev/2.1/v/value_error

Because of the strict=True I suspect there will be some Lists that we will want to change to FloatArrayLike (or create a generic CheckedArrayLike that allows arbitrary dtypes or something) but this is maybe a good enough start!

Closes #766

larsoner commented 1 year ago

Already found either a usage error or documentation / type-hinting error:

mne_bids_pipeline/_config_import.py:411: in _pydantic_validate
    raise ValueError(str(err)) from None
E   ValueError: 1 validation error for user configuration from /home/circleci/project/mne_bids_pipeline/tests/configs/config_ERP_CORE.py
E   eeg_template_montage
E     Input should be a valid string [type=string_type, input_value=<DigMontage | 0 extras (h...fiducials, 343 channels>, input_type=DigMontage]
E       For further information visit https://errors.pydantic.dev/2.1/v/string_type

Changing to just be eeg_template_montage = "standard_1005" but maybe someday we want to extend support to arbitrary DigMontage...

hoechenberger commented 1 year ago

Very nice otherwise!!

dengemann commented 1 year ago

I don’t remember. Probably it fixed something with some version of MNE. You can remove it or keep it and we see what happens :)

On Tue, 15 Aug 2023 at 19:10, Eric Larson @.***> wrote:

@.**** commented on this pull request.

In mne_bids_pipeline/tests/configs/config_ERP_CORE.py https://github.com/mne-tools/mne-bids-pipeline/pull/779#discussion_r1294889663 :

@@ -50,7 +49,7 @@

Suppress "Data file name in EEG.data (sub-019_task-ERN_eeg.fdt) is incorrect..."

read_raw_bids_verbose = "error"

-eeg_template_montage = mne.channels.make_standard_montage("standard_1005") +eeg_template_montage = "standard_1005"

Ahh, doing a bit of git digging I can see #407 https://github.com/mne-tools/mne-bids-pipeline/pull/407 by @dengemann https://github.com/dengemann so it was indeed intentional... I'll add support back in

— Reply to this email directly, view it on GitHub https://github.com/mne-tools/mne-bids-pipeline/pull/779#discussion_r1294889663, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOR7CTKCTHOMVSMSRGUID3XVOURBANCNFSM6AAAAAA3QGKJGQ . You are receiving this because you were mentioned.Message ID: @.***>