NeurodataWithoutBorders / pynwb

A Python API for working with Neurodata stored in the NWB Format
https://pynwb.readthedocs.io
Other
175 stars 85 forks source link

Tests do not run with clean state of pynwb #446

Open JFPerkins opened 6 years ago

JFPerkins commented 6 years ago

1) Bug

Changes to the global state that is tracked in the global state variables __TYPE_MAP and __NS_CATALOG persists across tests. This means that any tests that run after other tests that change the state (such as extension tests) are running with those changes.

The changing state can be observed by inserting:

import pynwb
logging.info("loaded namespaces %s", pynwb.get_type_map().namespace_catalog.namespaces)

Into the run_test_suite and run_example_tests driver functions in test.py.

Observed (condensed) output when run against dev branch:

======================================================================
2018-04-06 15:28:42,164 - INFO - running form unit tests
======================================================================
2018-04-06 15:28:44,762 - INFO - loaded namespaces ('core',)
...
======================================================================
2018-04-06 15:28:45,164 - INFO - running pynwb unit tests
======================================================================
2018-04-06 15:28:45,174 - INFO - loaded namespaces ('core',)
...
======================================================================
2018-04-06 15:28:45,394 - INFO - running example tests
======================================================================
2018-04-06 15:28:45,419 - INFO - loaded namespaces ('core', 'pynwb_test_extension1', 'pynwb_test_extension')
======================================================================
...
======================================================================
2018-04-06 15:28:45,907 - INFO - running integration tests
======================================================================
2018-04-06 15:28:45,937 - INFO - loaded namespaces ('core', 'pynwb_test_extension1', 'pynwb_test_extension', 'mylab')

Checklist

oruebel commented 2 years ago

@ajtritt is this still an issue or can this be closed?

ajtritt commented 2 years ago

Yes, I think this is no longer an issue. @rly can you confirm?

rly commented 2 years ago

This is still an issue, and has been raised by the DANDI developers. Loading an extension at one point will keep that extension loaded within the same Python execution (e.g., within a test suite or in a script that loops through dandisets and loads files). We have discussed resolving this in several ways in https://docs.google.com/document/d/1DB1dg6PEDW40Iafj-kX-y_OZJsqyne7Edah82n17zoo/edit

bendichter commented 2 years ago

Maybe if the namespace is loaded using NWBHDF5IO as a context, then whatever namespaces were loaded during __enter__ and be removed during __exit__?

rly commented 2 years ago

Actually, (I had to check this because I really did not know), this is already partially the case! NWBHDF5IO(..., load_namespaces=True) loads the extension namespaces in a type map that is local to the IO object. It does not influence future loads.

However, NWBHDF5IO(..., load_namespaces=True) will not load an older version of NWB core or hdmf-common into the local type map. That can be changed, but it might mess with the mapping between classes and older types, especially if the file is opened in append mode.

from pynwb import load_namespaces
load_namespaces(...)

which is used in the test suite, still loads it globally. And this is needed to write a file with an extension.