should pybids more lenient or at least get a dedicated exception like NonCompliantBIDSError

[yarikoptic]

ATM pybids immediately throws a generic ValueError if found contained derivatives dataset is not fully compliant:

/tmp > datalad install https://github.com/OpenNeuroDatasets/ds001868 && cd ds001868
[INFO   ] Cloning https://github.com/OpenNeuroDatasets/ds001868 [1 other candidates] into '/tmp/ds001868' 
[INFO   ]   Remote origin not usable by git-annex; setting annex-ignore                                         
[INFO   ] access to 1 dataset sibling s3-PRIVATE not auto-enabled, enable with:
| 		datalad siblings -d "/tmp/ds001868" enable -s s3-PRIVATE 
install(ok): /tmp/ds001868 (dataset)

/tmp/ds001868 > python -c 'from bids import BIDSLayout; b=BIDSLayout(".", derivatives=True)' 
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/yoh/proj/bids/pybids/bids/layout/layout.py", line 227, in __init__
    index_metadata=index_metadata)
  File "/home/yoh/proj/bids/pybids/bids/layout/layout.py", line 555, in add_derivatives
    raise ValueError("Every valid BIDS-derivatives dataset must "
ValueError: Every valid BIDS-derivatives dataset must have a PipelineDescription.Name field set inside dataset_description.json.

But I wonder

• if this should be a warning and not error (optionally or not)
• there should be a dedicated exception (e.g. NonCompliantBIDSError(ValueError) and NonCompliantDerivativesBIDSError(NonCompliantBIDSError)) which would be thrown here instead of a generic ValueError which makes it fragile to handle such cases programmatically

[effigies]

I'm 👍 for a sensible set of exceptions.

BIDSError
 - BIDSValidationError
   - DerivativeValidationError
   - ... (other optional BEPs?)

That said, I'm pretty sure you can use strict=False to avoid seeing that error.

[tyarkoni]

Also +1 for a hierarchy of exceptions—feel free to open a PR!

[tyarkoni]

My thinking on this has changed some. I spent some time a while back looking into it, and it seems like people are split pretty much 50-50 on whether best practice is to implement custom exceptions for every package, or to avoid it like the plague unless there's some critical reason for it. I don't have a strong feeling one way or the other, but the tiebreaker is that it would involve a fair amount of work to decide on a hierarchy, implement it, and then sweep through the codebase and replace the generic exceptions. Unless someone wants to volunteer to take this on, I propose to close this.

[tyarkoni]

Actually, I'm just going to go ahead and close it, to feed my issue-decluttering roll. If someone wants to volunteer to take this on, they can re-open it.

[yarikoptic]

. I spent some time a while back looking into it, and it seems like people are split pretty much 50-50 on whether best practice is to implement custom exceptions for every package

could you please show me some argumentation against "implementing custom exceptions"? note that here it was not really about "implementing" but rather "defining" as subclasses of base exceptions. I will now prep a basic PR to kick start the effort ;)

[tyarkoni]

@yarikoptic there are some reasonable arguments both ways here. But mainly I'm swayed by huge packages like numpy and pandas, which define very few custom exceptions, and largely rely on ValueError and TypeError. I can't say I've ever felt like having more exception classes in those packages would improve clarity much; at the end of the day, the error message is still the thing that's going to help you figure out what's going on.

That said, as I said above, I'm fine adding them if you do the work, which you've started to in #615. I just worry that we'll now proceed to argue for hours over what to include/rename/remove, whereas I don't think having a BIDSDerivativesValidationError class is going to cumulatively save us much development time. But I guess we'll see.