-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade validation methods - make pynwb.validate behave similarly for io and paths #1911
base: dev
Are you sure you want to change the base?
Conversation
) | ||
|
||
if io is not None: | ||
namespace_dependencies = io.load_namespaces(namespace_catalog=catalog, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if there is a better way to load the namespaces from the io object, I realize this is HDF5 specific, but it looks like the original namespace loading was as well? I was wondering how this works for Zarr validation?
self.assertEqual(fake_out.getvalue(), "") | ||
|
||
def test_validate_io_and_path_same(self): | ||
"""Test that validating a file with an io object and a path return the same results.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if all of these are necessary to include, but wanted to double check they were all equivalent
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1911 +/- ##
==========================================
+ Coverage 91.88% 92.00% +0.11%
==========================================
Files 27 27
Lines 2651 2663 +12
Branches 692 694 +2
==========================================
+ Hits 2436 2450 +14
+ Misses 142 141 -1
+ Partials 73 72 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
||
if status == 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from what I could tell, this status check was never reset to 0, so if any of the paths return a nonzero status, _validate_helper
would not be run on any subsequent paths. Is that the intended behavior? Or maybe I am misunderstanding something.
Motivation
Address #1807 (related to other validation upgrades described in #1808). This is a breaking change for the next major release.
The expected behavior is for
validate
to return the same output whetherpaths
orio
is provided. This was previously discussed here, where the 'correct' behavior for the example below is to return: "The namespace 'core' is included by the namespace 'ndx-testextension'. Please validate against that namespace instead".How to test the behavior?
Checklist
flake8
from the source directory.