Skip to content

Conversation

yarikoptic
Copy link
Contributor

@yarikoptic yarikoptic commented Sep 18, 2025

I think current behavior violates the principle of the least surprise: I ran validation while providing custom schema source, it executed with many warnings and errors reported to the screen. It is only after scrolling far up, I saw that actually my specification was wrong and operation resorted to built-in schema.

Moreover, having varying results depending on network flukes is also not a good behavior.

TODOs (if agreed in principle)

  • add changelog
  • fix or add a test (there should be a test for such behavior)

@yarikoptic yarikoptic requested a review from effigies September 18, 2025 21:15
@yarikoptic yarikoptic changed the title Throw and error, do not just log an error, if provided schema is not legit Throw an error, do not just log an error, if provided schema is not legit Sep 18, 2025
Copy link
Contributor

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been bitten by this. I can't remember why we decided this was a good idea, but I disagree with past us.

…readable

I think current behavior violates the principle of the least surprise: I ran
validation while providing custom schema source, it executed with many warnings
and errors reported to the screen. It is only after scrolling far up, I saw
that actually my specification was wrong and operation resorted to built-in schema.

amend: removed comment "version is ignored when the network cannot be accessed"
@effigies
Copy link
Contributor

What do you think about the case where someone has BIDS_SCHEMA in their environment but doesn't know it. Should we indicate where the URL came from?

@yarikoptic
Copy link
Contributor Author

that might come useful although I do not think any other software I know bothers to tell apart where config value came from.

If to add -- do you feel it is better as error post-processing (check again if env var available) or pair with

and add some schemaSourceMethod = CLIOption|envvar (feels overweight FWIW)?

@effigies effigies merged commit 038e8bd into bids-standard:main Sep 19, 2025
27 checks passed
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.

2 participants