Structured validation results #1104
Conversation
Codecov ReportBase: 88.05% // Head: 88.04% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1104 +/- ##
==========================================
- Coverage 88.05% 88.04% -0.01%
==========================================
Files 73 73
Lines 8586 8727 +141
==========================================
+ Hits 7560 7684 +124
- Misses 1026 1043 +17
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
This pull request introduces 1 alert when merging 72a51fb into ef097cc - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 2c78bcf into e74c5ac - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 7a5239c into e74c5ac - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 0f44a23 into e74c5ac - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging afa56ac into e74c5ac - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging af9f3db into e74c5ac - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging f92b1e3 into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 0508f49 into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 90f2819 into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 7075a7c into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging d0753e8 into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 9576526 into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging bf0e17e into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 8928e26 into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 5aa69e9 into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 7e54f3c into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 8510722 into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 2be5aaa into 17e0b04 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 1b15e6d into 17e0b04 - view on LGTM.com new alerts:
|
Still using 2 separate functions, merging of validation functions best for subsequent PR
and renamed tests for more consistency
|
@jwodder thank you, yes, the error no longer happens. Apparently there was another one, but I fixed that as well. @yarikoptic ready for merge? |
|
ok, let's merge and then tune up from there. Thanks @TheChymera and @jwodder for the reviews. |
| selected_dataset = os.path.join(bids_examples, dataset) | ||
| validation_result = validate_bids(selected_dataset, report=True) | ||
| for i in validation_result: | ||
| assert not hasattr(i, "severtiy") |
There was a problem hiding this comment.
@TheChymera Is this typo deliberate? If it is, what's the point? If it's not, shouldn't you instead be testing that the attribute is None?
There was a problem hiding this comment.
@jwodder thank you for spotting this, indeed it was an error, and further, it was giving a false positive arising from an upstream oversight. Will sumbit a fix PR once the reference data is fixed, since currently fixing this would fail due to the data being broken.
Closes #943.