Add test_gallery.py and debug python_3.7 gallery tests

[mavaylon1]

No description provided.

[mavaylon1]

#52

[codecov-commenter]

Codecov Report

Base: 82.15% // Head: 80.59% // Decreases project coverage by -1.56% ⚠️

Coverage data is based on head (54d95d0) compared to base (e3d063a).
Patch coverage: 29.16% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev      #53      +/-   ##
==========================================
- Coverage   82.15%   80.59%   -1.57%     
==========================================
  Files           9       10       +1     
  Lines        2365     2437      +72     
==========================================
+ Hits         1943     1964      +21     
- Misses        422      473      +51     

Impacted Files	Coverage Δ
test_gallery.py	29.16% <29.16%> (ø)

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

[mavaylon1]

1. Even though we are switching the gallery tests to test_gallery.py, the current use of "python test.py --example" leads to different results. On my local conda env with python 3.7, "python test.py --example" passes; however, here it returns multiple "File not Found" errors.
2. Running "python test_gallery.py" returns the following error for "plot_convert_nwb_hdf5.py" locally (refer to image). This is a user warning, should I add an exception? @oruebel
   

[oruebel]

2. Running "python test_gallery.py" returns the following error for "plot_convert_nwb_hdf5.py" locally (refer to image). This is a user warning, should I add an exception?

The user warning that The number of frame indices in `starting_frame should have the same length as external_file I think may be the same as this issue #45 However, it could also be a bug in the tutorial. It would be nice to raise errors in the gallery test when unexpected warnings occur, however, I'm not sure right now how to configure this, since there may be some warnings that are expect to occur.

[oruebel]

2. Running "python test_gallery.py" returns the following error for "plot_convert_nwb_hdf5.py" locally (refer to image). This is a user warning, should I add an exception?

The strange thing is though, that this particular warning does not seem to happen on ReadTheDocs https://hdmf-zarr.readthedocs.io/en/dev/tutorials/plot_convert_nwb_hdf5.html Which version of PyNWB are you using? I'm just wondering whether this a new check on the dev branch of PyNWB so that you are seeing it on your system but it does not yet appear when using the release version of PyNWB.

[rly]

This line at the end of plot_nwb_zarrio.py

hdmf-zarr/docs/gallery/plot_nwb_zarrio.py

Line 155 in 6841196

os.chdir(os.path.abspath(os.path.join(os.getcwd(), "../")))

changes the current directory of the Python execution which messes up finding the other gallery files.

I would suggest changing the current directory back at the end of that script.

This does not happen in test_gallery.py I think because the _import_from_file function is different and more robust to changes in file location.

@oruebel The warning was added in PyNWB 2.1.1: NeurodataWithoutBorders/pynwb#1516
The requirements.txt file in this repo pins PyNWB to version 2.0.1 and this is used by readthedocs.

[rly]

I think it is fine to move to using test_gallery.py for gallery tests and pytest for all other tests

[oruebel]

changes the current directory of the Python execution which messes up finding the other gallery files

Good catch! To be honest, this part of the script

hdmf-zarr/docs/gallery/plot_nwb_zarrio.py

Lines 144 to 158 in 6841196

	###############################################################################
	# Test opening with the absolute path instead
	# -------------------------------------------
	with NWBZarrIO(path=absolute_path, mode="r") as io:
	infile = io.read()
	###############################################################################
	# Test changing the current directory
	# ------------------------------------
	import os
	os.chdir(os.path.abspath(os.path.join(os.getcwd(), "../")))
	with NWBZarrIO(path=absolute_path, mode="r") as io:
	infile = io.read()

should actually be changed to a unit instead. The reason for changing the current directory here was because I was debugging the issue with links and changing the current directory was a way to trigger the issue without copying files. For now, I would suggest the following: 1) Remove those lines from the gallery, 2) Open an issue to add a test for this case, basically trimming down the notebook and making it into a unit test, and 3) actually implement the unit test.

[oruebel]

I have now removed the problematic lines from the plot_nwb_zarrio.py tutorial and create a new issue #58 to add a test to cover this case in the regular test suite. It looks like with this change, the 3.7 pipelines are not passing.

[mavaylon1]

If I downgrade to pynwb 2.1.0 the gallery tests pass. This seems to be a new warning/error coming from pynwb 2.2.0. For now I can downgrade pynwb to be greater than 2.0.0 but less than or equal to 2.1.0. That way we can merge test_gallery and look create a ticket for debugging the tutorials to whatever pynwb 2.2.0 needs. @oruebel

[oruebel]

. This seems to be a new warning/error coming from pynwb 2.2.0.

Just to clarify, are you referring to the following warning:

UserWarning: OpticalSeries 'StimulusPresentation_encoding': The number of frame indices in 'starting_frame' should have the same length as 'external_file'.`

or are there other warnings/errors? This particular warning is due to an error in the NWB file from DANDIset 000207 we are using in the tutorial:

hdmf-zarr/docs/gallery/plot_convert_nwb_hdf5.py

Lines 17 to 18 in 6841196

	dandiset_id = '000207'
	filepath = "sub-1/sub-1_ses-1_ecephys+image.nwb" # 5 MB file

The file is using the starting_frame field wrong. The correct usage for this field was clarified in the schema and API but I assume this file was created before than and used the field wrong. This is now raises a warning in PyNWB. To address this issue we should probably use a different dataset in the tutorial. Rather than downloading a dataset from DANDI it may be best to use the PyNWB test/mock function to create a dummy NWB file and use that instead in the test.

[mavaylon1]

@oruebel
My bad I thought this issue was in the chain of comments above. I was referring to the image attached here for plot_convert_nwb.py

[oruebel]

@mavaylon1 #56 fixes the issue in conversion tutorial and another bug in the ZarrIO tutorial.

[mavaylon1]

@oruebel so we need the gallery test for 3.10 to pass. Let's merge this with pynwb==2.1.0, and then change it to be more general in #56.

[mavaylon1]

I'll take a look at the windows gallery since it is throwing an error not related to the tutorial you updated. However, the sphinx test for external links is throwing an error due to the updated conversion tutorial. @oruebel

[oruebel]

@mavaylon1 does the linkcheck run a different setup? It is strange that the tutorial is failing there but in none of the other checks. I'll take a look later today.

[oruebel]

It looks like some of the failures are due to DANDI download. Since the file is small, we could just add it to the repo and not download from DANDI here.