Skip to content

Commit

Permalink
Use NWBInspector when testing livescript tutorials (#618)
Browse files Browse the repository at this point in the history
* Add function for running nwbinspector on tutorial files during testing #484

* Fix intro tutorial

* Add nwbInspector check to ignore list in TutorialTest

* Fix timestamps with wrong length in Behavior tutorial

* Update behavior tutorial.

- Add note about why dimension of SpatialSeries data is transposed wrt the type specification
- Fix typos

* Update images tutorial

- Add explanation of why arrays are transposed relative to the type documentation
- Fix dimension order of AbstractFeatureSeries

* Add 'cell_id' to types.core.IntracellularElectrode in Icephys tutorial

* Update run_tests.yml

* Update TutorialTest.m

* Update TutorialTest.m

* Update TutorialTest.m

* Try o set up python path

* ...

* ...

* ..

* ...

* test

* ...

* Update TutorialTest.m

* Fix TutorialTest

* Tests are working on GItHub Actions

* Update TutorialTest.m

* Fix

* Update untypedSetTest.m

Suppress output in test

* Add fixture for clearing generated types when running tests

* Fix typo

* Improve Fixture description and improve function names

* Use OutOfProcess execution mode for python in matlan for github action

- Simplifies the reading of nwb files using pynwb in TutorialTest

* Update run_tests.yml

* Added sentence about using timestamps

* Ignore NWBInspector result for specific dataset in ecephys tutorial
  • Loading branch information
ehennestad authored Nov 14, 2024
1 parent 7caa336 commit 21bd143
Show file tree
Hide file tree
Showing 18 changed files with 571 additions and 482 deletions.
16 changes: 16 additions & 0 deletions +tests/+fixtures/ResetGeneratedTypesFixture.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
classdef ResetGeneratedTypesFixture < matlab.unittest.fixtures.Fixture
% ResetGeneratedTypesFixture - Fixture for resetting generated NWB classes.
%
% ResetGeneratedTypesFixture clears all the generated classes for NWB
% types from the matnwb folder. When the fixture is set up, all generated
% class files for NWB types are deleted. When the fixture is torn down,
% generateCore is called to regenerate the classes for NWB types of the
% latest NWB version

methods
function setup(fixture)
fixture.addTeardown( @generateCore )
nwbClearGenerated()
end
end
end
10 changes: 8 additions & 2 deletions +tests/+sanity/GenerationTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@

methods (TestClassSetup)
function setupClass(testCase)
rootPath = fullfile(fileparts(mfilename('fullpath')), '..', '..');
testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath));

import matlab.unittest.fixtures.PathFixture
import tests.fixtures.ResetGeneratedTypesFixture

rootPath = tests.util.getProjectDirectory();
testCase.applyFixture( PathFixture(rootPath) );

testCase.applyFixture( ResetGeneratedTypesFixture );
end
end

Expand Down
2 changes: 1 addition & 1 deletion +tests/+system/PyNWBIOTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function testInFromPyNWB(testCase)

methods
function [status, cmdout] = runPyTest(testCase, testName)
setenv('PYTHONPATH', fileparts(mfilename('fullpath')));
tests.util.addFolderToPythonPath( fileparts(mfilename('fullpath')) )

envPath = fullfile('+tests', 'env.mat');
if 2 == exist(envPath, 'file')
Expand Down
255 changes: 190 additions & 65 deletions +tests/+unit/TutorialTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,27 @@
%
% This test will test most tutorial files (while skipping tutorials with
% dependencies) If the tutorial creates an nwb file, the test will also try
% to open this with pynwb.
%
% Note:
% - Requires MATLAB XXXX to run py.* commands.
% - pynwb must be installed in the python environment returned by
% pyenv()
% to open this with pynwb and run nwbinspector on the file.

% Notes:
% - Requires MATLAB 2019b or later to run py.* commands.
%
% - pynwb must be installed in the python environment returned by pyenv()
%
% - Running NWBInspector as a Python package within MATLAB on GitHub runners
% currently encounters compatibility issues between the HDF5 library and
% h5py. As a workaround in this test, the CLI interface is used to run
% NWBInspector and the results are manually parsed. This approach is not
% ideal, and hopefully can be improved upon.

properties
MatNwbDirectory
end

properties (Constant)
NwbInspectorSeverityLevel = 1
end

properties (TestParameter)
% TutorialFile - A cell array where each cell is the name of a
% tutorial file. testTutorial will run on each file individually
Expand All @@ -30,12 +40,22 @@

% SkippedFiles - Name of exported nwb files to skip reading with pynwb
SkippedFiles = {'testFileWithDataPipes.nwb'} % does not produce a valid nwb file

% PythonDependencies - Package dependencies for running pynwb tutorials
PythonDependencies = {'nwbinspector'}
end

properties (Access = private)
NWBInspectorMode = "python"
end

methods (TestClassSetup)
function setupClass(testCase)

import tests.fixtures.ResetGeneratedTypesFixture

% Get the root path of the matnwb repository
rootPath = getMatNwbRootDirectory();
rootPath = tests.util.getProjectDirectory();
tutorialsFolder = fullfile(rootPath, 'tutorials');

testCase.MatNwbDirectory = rootPath;
Expand All @@ -44,29 +64,16 @@ function setupClass(testCase)
testCase.applyFixture(matlab.unittest.fixtures.PathFixture(rootPath));
testCase.applyFixture(matlab.unittest.fixtures.PathFixture(tutorialsFolder));

% Note: The following seems to not be working on the azure pipeline
% Keep for reference

% % % Make sure pynwb is installed in MATLAB's Python Environment
% % args = py.list({py.sys.executable, "-m", "pip", "install", "pynwb"});
% % py.subprocess.check_call(args);
% %
% % % Add pynwb to MATLAB's python environment path
% % pynwbPath = getenv('PYNWB_PATH');
% % if count(py.sys.path, pynwbPath) == 0
% % insert(py.sys.path,int32(0),pynwbPath);
% % end

% % Alternative: Use python script for reading file with pynwb
setenv('PYTHONPATH', fileparts(mfilename('fullpath')));

nwbClearGenerated()
end
end
% Check if it is possible to call py.nwbinspector.* functions.
% When running these tests on Github Actions, calling
% py.nwbinspector does not work, whereas the CLI can be used instead.
try
py.nwbinspector.is_module_installed('nwbinspector');
catch
testCase.NWBInspectorMode = "CLI";
end

methods (TestClassTeardown)
function tearDownClass(testCase) %#ok<MANU>
%generateCore()
testCase.applyFixture( ResetGeneratedTypesFixture );
end
end

Expand All @@ -79,64 +86,182 @@ function setupMethod(testCase)

methods (Test)
function testTutorial(testCase, tutorialFile) %#ok<INUSD>
% Intentionally capturing output, in order for tests to cover
% code which overloads display methods for nwb types/objects.
C = evalc( 'run(tutorialFile)' ); %#ok<NASGU>
testCase.testReadTutorialNwbFileWithPynwb()

testCase.readTutorialNwbFileWithPynwb()
testCase.inspectTutorialFileWithNwbInspector()
end
end

methods
function testReadTutorialNwbFileWithPynwb(testCase)
function readTutorialNwbFileWithPynwb(testCase)

% Retrieve all files generated by tutorial
nwbListing = dir('*.nwb');
nwbFileNameList = testCase.listNwbFiles();
for nwbFilename = nwbFileNameList
try
io = py.pynwb.NWBHDF5IO(nwbFilename);
nwbObject = io.read();
testCase.verifyNotEmpty(nwbObject, 'The NWB file should not be empty.');
io.close()
catch ME
error(ME.message)
end
end
end

function inspectTutorialFileWithNwbInspector(testCase)
% Retrieve all files generated by tutorial
nwbFileNameList = testCase.listNwbFiles();
for nwbFilename = nwbFileNameList
if testCase.NWBInspectorMode == "python"
results = py.list(py.nwbinspector.inspect_nwbfile(nwbfile_path=nwbFilename));
results = testCase.convertNwbInspectorResultsToStruct(results);
elseif testCase.NWBInspectorMode == "CLI"
[~, m] = system(sprintf('nwbinspector %s --levels importance', nwbFilename));
results = testCase.parseNWBInspectorTextOutput(m);
end

if isempty(results)
return
end

results = testCase.filterNWBInspectorResults(results);
% T = struct2table(results); disp(T)

for j = 1:numel(results)
testCase.verifyLessThan(results(j).importance, testCase.NwbInspectorSeverityLevel, ...
sprintf('Message: %s\nLocation: %s\n File: %s\n', ...
string(results(j).message), results(j).location, results(j).filepath))
end
end
end
end

for i = 1:numel(nwbListing)
nwbFilename = nwbListing(i).name;
if any(strcmp(nwbFilename, tests.unit.TutorialTest.SkippedFiles))
continue
methods (Access = private)
function nwbFileNames = listNwbFiles(testCase)
nwbListing = dir('*.nwb');
nwbFileNames = string( {nwbListing.name} );
nwbFileNames = setdiff(nwbFileNames, testCase.SkippedFiles);
assert(isrow(nwbFileNames), 'Expected output to be a row vector')
if ~isscalar(nwbFileNames)
if iscolumn(nwbFileNames)
nwbFileNames = transpose(nwbFileNames);
end
end
end
end

methods (Static)
function resultsOut = convertNwbInspectorResultsToStruct(resultsIn)

resultsOut = tests.unit.TutorialTest.getEmptyNwbInspectorResultStruct();

C = cell(resultsIn);
for i = 1:numel(C)
resultsOut(i).importance = double( py.getattr(C{i}.importance, 'value') );
resultsOut(i).severity = double( py.getattr(C{i}.severity, 'value') );

try
resultsOut(i).location = string(C{i}.location);
catch
resultsOut(i).location = "N/A";
end

resultsOut(i).message = string(C{i}.message);
resultsOut(i).filepath = string(C{i}.file_path);
resultsOut(i).check_name = string(C{i}.check_function_name);
end
end

function resultsOut = parseNWBInspectorTextOutput(systemCommandOutput)
resultsOut = tests.unit.TutorialTest.getEmptyNwbInspectorResultStruct();

importanceLevels = containers.Map(...
["BEST_PRACTICE_SUGGESTION", ...
"BEST_PRACTICE_VIOLATION", ...
"CRITICAL", ...
"PYNWB_VALIDATION", ...
"ERROR"], 0:4 );

lines = splitlines(systemCommandOutput);
count = 0;
for i = 1:numel(lines)
% Example line:
% '.0 Importance.BEST_PRACTICE_VIOLATION: behavior_tutorial.nwb - check_regular_timestamps - 'SpatialSeries' object at location '/processing/behavior/Position/SpatialSeries'
% ^2 ^1 ^2 ^ ^ ^ 3
% [-----------importance------------] [------filepath------] [------check_name------] [-----------------location----------------]
% Splitting and components is exemplified above.

if ~isempty(regexp( lines{i}, '^\.\d{1}', 'once' ) )
count = count+1;
% Split line into separate components
splitLine = strsplit(lines{i}, ':');
splitLine = [...
strsplit(splitLine{1}, ' '), ...
strsplit(splitLine{2}, '-') ...
];

resultsOut(count).importance = importanceLevels( extractAfter(splitLine{2}, 'Importance.') );
resultsOut(count).filepath = string(strtrim( splitLine{3} ));
resultsOut(count).check_name = string(strtrim(splitLine{4} ));
try
io = py.pynwb.NWBHDF5IO(nwbListing(i).name);
nwbObject = io.read();
testCase.verifyNotEmpty(nwbObject, 'The NWB file should not be empty.');
io.close()

catch ME
if strcmp(ME.identifier, 'MATLAB:undefinedVarOrClass') && ...
contains(ME.message, 'py.pynwb.NWBHDF5IO')

pythonExecutable = tests.util.getPythonPath();
cmd = sprintf('"%s" -B -m read_nwbfile_with_pynwb %s',...
pythonExecutable, nwbFilename);

status = system(cmd);
if status ~= 0
error('Failed to read NWB file "%s" using pynwb', nwbFilename)
end
else
rethrow(ME)
end
locationInfo = strsplit(splitLine{end}, 'at location');
resultsOut(count).location = string(strtrim(eval(locationInfo{2})));
catch
resultsOut(count).location = 'N/A';
end
resultsOut(count).message = string(strtrim(lines{i+1}));
end
end
end

catch ME
error(ME.message)
%testCase.verifyFail(sprintf('Failed to read file %s with error: %s', nwbListing(i).name, ME.message));
function emptyResults = getEmptyNwbInspectorResultStruct()
emptyResults = struct(...
'importance', {}, ...
'severity', {}, ...
'location', {}, ...
'filepath', {}, ...
'check_name', {}, ...
'ignore', {});
end

function resultsOut = filterNWBInspectorResults(resultsIn)
CHECK_IGNORE = [...
"check_image_series_external_file_valid", ...
"check_regular_timestamps"
];

for i = 1:numel(resultsIn)
resultsIn(i).ignore = any(strcmp(CHECK_IGNORE, resultsIn(i).check_name));

% Special cases to ignore
if resultsIn(i).location == "/acquisition/ExternalVideos" && ...
resultsIn(i).check_name == "check_timestamps_match_first_dimension"
resultsIn(i).ignore = true;
elseif resultsIn(i).location == "/acquisition/SpikeEvents_Shank0" && ...
resultsIn(i).check_name == "check_data_orientation"
% Data for this example is actually longer in another dimension
% than time.
resultsIn(i).ignore = true;
end
end
resultsOut = resultsIn;
resultsOut([resultsOut.ignore]) = [];
end
end
end

function tutorialNames = listTutorialFiles()
% listTutorialFiles - List names of all tutorial files (exclude skipped files)
rootPath = getMatNwbRootDirectory();
L = dir(fullfile(rootPath, 'tutorials'));
rootPath = tests.util.getProjectDirectory();
L = cat(1, ...
dir(fullfile(rootPath, 'tutorials', '*.mlx')), ...
dir(fullfile(rootPath, 'tutorials', '*.m')) ...
);

L( [L.isdir] ) = []; % Ignore folders
tutorialNames = setdiff({L.name}, tests.unit.TutorialTest.SkippedTutorials);
end

function folderPath = getMatNwbRootDirectory()
folderPath = fileparts(fileparts(fileparts(mfilename('fullpath'))));
end
10 changes: 6 additions & 4 deletions +tests/+unit/untypedSetTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ function testCreateSetFromNvPairsPlusFunctionHandle(testCase)
end

function testDisplayEmptyObject(testCase)
emptyUntypedSet = types.untyped.Set();
disp(emptyUntypedSet)
emptyUntypedSet = types.untyped.Set(); %#ok<NASGU>
C = evalc( 'disp(emptyUntypedSet)' );
testCase.verifyClass(C, 'char')
end

function testDisplayScalarObject(testCase)
scalarSet = types.untyped.Set('a',1)
disp(scalarSet)
scalarSet = types.untyped.Set('a', 1); %#ok<NASGU>
C = evalc( 'disp(scalarSet)' );
testCase.verifyClass(C, 'char')
end

function testGetSetSize(testCase)
Expand Down
13 changes: 13 additions & 0 deletions +tests/+util/addFolderToPythonPath.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
function addFolderToPythonPath(folderPath)
pythonPath = getenv('PYTHONPATH');
if isempty(pythonPath)
updatedPythonPath = folderPath;
else
if ~contains(pythonPath, folderPath)
updatedPythonPath = strjoin({pythonPath, folderPath}, pathsep);
else
return
end
end
setenv('PYTHONPATH', updatedPythonPath);
end
3 changes: 3 additions & 0 deletions +tests/+util/getProjectDirectory.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function projectDirectory = getProjectDirectory()
projectDirectory = fullfile(fileparts(mfilename('fullpath')), '..', '..');
end
Loading

0 comments on commit 21bd143

Please sign in to comment.