Skip to content

Conversation

@happy5214
Copy link
Contributor

This cleans up various minor issues, and it should hopefully deal with the issues in #145 (though, as I couldn't reproduce the original issue, I couldn't test that).

@happy5214
Copy link
Contributor Author

It appears the tests are expecting two HED errors? I only know of one (a failed schema load due to an invalid specification), so I don't know what the other intended error was (perhaps the caught internal error from the spread syntax?). Please investigate.

@happy5214
Copy link
Contributor Author

Tagging @VisLab.

@effigies
Copy link
Contributor

I strongly suspect that the test checked for 2 issues because we were getting 2. We are generally deferring to you on what is expected for HED. Please feel free to update the expected number of issues in the test.

cc @rwblair in case you remember something different.

Since the "internal error" for the null pointer is no longer thrown,
there is now only one issue for an invalid schema spec on this test.
@happy5214
Copy link
Contributor Author

I confirmed the second issue is from the null pointer, which is no longer generated. I've decreased the issue counts to 1.

@VisLab
Copy link
Member

VisLab commented Feb 12, 2025

Looks good. Everything we discussed has been addressed.

@rwblair rwblair merged commit 49b8742 into bids-standard:main Feb 12, 2025
17 checks passed
@happy5214 happy5214 deleted the hed-module-fixes branch February 12, 2025 19:51
effigies added a commit to effigies/bids-validator that referenced this pull request Feb 14, 2025
rwblair added a commit that referenced this pull request Feb 14, 2025
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.

4 participants