Skip to content
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

Prepare for version 2.5.0 release #550

Merged
merged 42 commits into from
Apr 22, 2021
Merged

Prepare for version 2.5.0 release #550

merged 42 commits into from
Apr 22, 2021

Conversation

rly
Copy link
Contributor

@rly rly commented Mar 30, 2021

  • Updated changelog
  • Updated requirements
  • Updated CI
  • Add additional cache spec test and clean up some other tests

Some critical bugs were identified with the experimental namespace and class generation. Ideally, these will be resolved before this release.

CI notes:

  • h5py 2.10 did not release wheels for Python 3.9, so it has to be built from source using the HDF5 library. This is doable in Linux, but I cannot figure it out for Windows and MacOS on Azure.
  • On Python 3.9, we have to build h5py 2.10. If numpy 1.20 is installed, then the build of h5py 2.10 appears to be incompatible with numpy 1.19: ValueError: numpy.ndarray size changed, may indicate binary incompatibility. Expected 88 from C header, got 80 from PyObject . Upgrading to numpy 1.20 solves the issue but numpy 1.20 does not support Python 3.6, which we want to continue to support. For this reason, the python3.9 job fails. So users should note that when using Python 3.9 (which is not fully supported by HDMF), they should install a version of numpy first, and then build the wheels for h5py 2.10.
  • The pynwb-dev-python38 job fails because of a single test in pynwb that does not expect the hdmf-experimental namespace.

@rly rly requested a review from a team March 30, 2021 08:48
oruebel
oruebel previously approved these changes Mar 30, 2021
@codecov
Copy link

codecov bot commented Mar 30, 2021

Codecov Report

Merging #550 (1c23e7e) into dev (ecb260e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #550   +/-   ##
=======================================
  Coverage   84.73%   84.73%           
=======================================
  Files          39       39           
  Lines        7985     7985           
  Branches     1693     1693           
=======================================
  Hits         6766     6766           
  Misses        883      883           
  Partials      336      336           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ecb260e...1c23e7e. Read the comment docs.

@rly rly requested a review from oruebel March 30, 2021 20:41
@rly
Copy link
Contributor Author

rly commented Apr 10, 2021

There are three new test failures in PyNWB:

ERROR in test test_get_class (test_extension.TestExtension):
  Traceback (most recent call last):
    File "/home/circleci/project/pynwb/tests/unit/test_extension.py", line 52, in test_get_class
      type_map.get_container_cls(self.prefix, 'TetrodeSeries')
    File "/home/circleci/project/src/hdmf/utils.py", line 580, in func_call
      return func(args[0], **pargs)
    File "/home/circleci/project/src/hdmf/build/manager.py", line 505, in get_container_cls
      spec = self.__ns_catalog.get_spec(namespace, data_type)
    File "/home/circleci/project/src/hdmf/utils.py", line 580, in func_call
      return func(args[0], **pargs)
    File "/home/circleci/project/src/hdmf/spec/namespace.py", line 316, in get_spec
      raise KeyError("'%s' not a namespace" % namespace)
  KeyError: "'TetrodeSeries' not a namespace"

  ERROR in test test_lab_meta_auto (test_extension.TestExtension):
  Traceback (most recent call last):
    File "/home/circleci/project/pynwb/tests/unit/test_extension.py", line 129, in test_lab_meta_auto
      MyTestMetaData = get_class('MyTestMetaData', self.prefix)
    File "/home/circleci/project/src/hdmf/utils.py", line 584, in func_call
      return func(**pargs)
    File "/home/circleci/project/pynwb/src/pynwb/__init__.py", line 188, in get_class
      return __TYPE_MAP.get_container_cls(namespace, neurodata_type)
    File "/home/circleci/project/src/hdmf/utils.py", line 580, in func_call
      return func(args[0], **pargs)
    File "/home/circleci/project/src/hdmf/build/manager.py", line 505, in get_container_cls
      spec = self.__ns_catalog.get_spec(namespace, data_type)
    File "/home/circleci/project/src/hdmf/utils.py", line 580, in func_call
      return func(args[0], **pargs)
    File "/home/circleci/project/src/hdmf/spec/namespace.py", line 316, in get_spec
      raise KeyError("'%s' not a namespace" % namespace)
  KeyError: "'MyTestMetaData' not a namespace"

  ERROR in test test_load_namespace_with_reftype_attribute_check_autoclass_const (test_extension.TestExtension):
  Traceback (most recent call last):
    File "/home/circleci/project/pynwb/tests/unit/test_extension.py", line 77, in test_load_namespace_with_reftype_attribute_check_autoclass_const
      my_new_type = type_map.get_container_cls(self.prefix, 'my_new_type')
    File "/home/circleci/project/src/hdmf/utils.py", line 580, in func_call
      return func(args[0], **pargs)
    File "/home/circleci/project/src/hdmf/build/manager.py", line 505, in get_container_cls
      spec = self.__ns_catalog.get_spec(namespace, data_type)
    File "/home/circleci/project/src/hdmf/utils.py", line 580, in func_call
      return func(args[0], **pargs)
    File "/home/circleci/project/src/hdmf/spec/namespace.py", line 316, in get_spec
      raise KeyError("'%s' not a namespace" % namespace)
  KeyError: "'my_new_type' not a namespace"

@rly
Copy link
Contributor Author

rly commented Apr 22, 2021

Before release, there are two issues to be resolved:

Exception: VectorData (acquisition/UnitsTest/spike_times.resolution): encountered extra field

See #584 which should resolve it.

And two tests:
tests.integration.test_io.TestHDF5Writer testMethod=test_write_cache_spec
tests.integration.test_io.TestHDF5WriterWithInjectedFile testMethod=test_write_cache_spec

E                   AssertionError: {'gro[462 chars]], 'datasets': [{'shape': [None], 'dims': ['nu[1582 chars]t'}]} != {'gro[462 chars]], 'doc': 'DynamicTable container that support[1250 chars]t'}]}
E                     {'attributes': [{'dims': ['num_categories'],
E                                      'doc': 'The names of the categories in this '
E                                             'AlignedDynamicTable. Each category is represented by '
E                                             'one DynamicTable stored in the parent group. This '
E                                             'attribute should be used to specify an order of '
E                                             'categories and the category names must match the '
E                                             'names of the corresponding DynamicTable in the group.',
E                                      'dtype': 'text',
E                                      'name': 'categories',
E                                      'shape': [None]},
E                                     {'dims': ['num_columns'],
E                                      'doc': 'The names of the columns in this table. This should '
E                                             'be used to specify an order to the columns.',
E                                      'dtype': 'text',
E                                      'name': 'colnames',
E                                      'shape': [None]},
E                                     {'doc': 'Description of what is in this dynamic table.',
E                                      'dtype': 'text',
E                                      'name': 'description'}],
E                   -  'datasets': [{'dims': ['num_rows'],
E                   -                'doc': 'Array of unique identifiers for the rows of this '
E                   -                       'dynamic table.',
E                   -                'dtype': 'int',
E                   -                'name': 'id',
E                   -                'neurodata_type_inc': 'ElementIdentifiers',
E                   -                'shape': [None]},
E                   -               {'doc': 'Vector columns, including index columns, of this '
E                   -                       'dynamic table.',
E                   -                'neurodata_type_inc': 'VectorData',
E                   -                'quantity': '*'}],
E                      'doc': 'DynamicTable container that supports storing a collection of '
E                             'sub-tables. Each sub-table is a DynamicTable itself that is aligned '
E                             'with the main table by row index. I.e., all DynamicTables stored in '
E                             'this group MUST have the same number of rows. This type effectively '
E                             'defines a 2-level table in which the main data is stored in the main '
E                             'table implemented by this type and additional columns of the table '
E                             'are grouped into categories, with each category being represented by '
E                             'a separate DynamicTable stored within the group.',
E                      'groups': [{'doc': 'A DynamicTable representing a particular category for '
E                                         'columns in the AlignedDynamicTable parent container. The '
E                                         'table MUST be aligned with (i.e., have the same number of '
E                                         'rows) as all other DynamicTables stored in the '
E                                         'AlignedDynamicTable parent container. The name of the '
E                                         'category is given by the name of the DynamicTable and its '
E                                         'description by the description attribute of the '
E                                         'DynamicTable.',
E                                  'neurodata_type_inc': 'DynamicTable',
E                                  'quantity': '*'}],
E                      'neurodata_type_def': 'AlignedDynamicTable',
E                      'neurodata_type_inc': 'DynamicTable'}

where the AlignedDynamicTable spec that is loaded locally has its inherited 'id' and VectorData columns resolved (included) whereas the spec cached in the file does not.

Interestingly, this does not happen when running this test in hdmf-common only. It seems to happen only because NWB includes HDMF-common.

@rly
Copy link
Contributor Author

rly commented Apr 22, 2021

After #503 is merged and #547 is addressed, then we can go ahead with this minor release.

@rly
Copy link
Contributor Author

rly commented Apr 22, 2021

#547 can be addressed by setting a max version for ruamel.yaml that does not show the PendingDeprecationWarning, or if time permits, migrating the to-be-deprecated calls to the new format. It should be addressed in this release because it clutters the output on I/O and may confuse users.

@rly rly requested review from ajtritt, bendichter and oruebel and removed request for oruebel April 22, 2021 18:59
@oruebel
Copy link
Contributor

oruebel commented Apr 22, 2021

There are three new test failures in PyNWB:

ERROR in test test_get_class (test_extension.TestExtension):
  Traceback (most recent call last):

Is this going to be part of another PR?

@rly
Copy link
Contributor Author

rly commented Apr 22, 2021

There are three new test failures in PyNWB:

ERROR in test test_get_class (test_extension.TestExtension):
  Traceback (most recent call last):

Is this going to be part of another PR?

These errors are addressed in NeurodataWithoutBorders/pynwb#1354

@rly rly merged commit 5d903ab into dev Apr 22, 2021
@rly rly deleted the prepare_2.5.0 branch April 22, 2021 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants