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

Revert breaking change of TypeMap.get_container_cls function header #590

Merged
merged 3 commits into from
Apr 23, 2021

Conversation

rly
Copy link
Contributor

@rly rly commented Apr 23, 2021

Motivation

#555 introduced a breaking change in the function header for TypeMap.get_container_cls, where the namespace and data_type arguments were swapped and namespace was made optional. Initially, this was thought to be OK for a minor release because this undocumented function is used internally within HDMF and externally only by pynwb.get_class. Given the tight relationship between our development of HDMF and PyNWB, we thought we would simply update PyNWB to address this breaking change. Although we are doing that, the versions of HDMF (>=2.1.0, <3) allowed by the current release of PyNWB (1.4.0) rely on the fact that all releases of HDMF up to version 3 would be compatible with this release of PyNWB. In other words, dependency versioning in PyNWB relies on the policy that breaking changes would occur only in major version bumps. Since that policy was not followed strictly, now PyNWB 1.4.0 is broken for all versions of HDMF 2.5.0 and up and users would not know that when they update HDMF but not PyNWB.

Proposed solution (this PR): Effectively revert these breaking changes for now and re-add them for the next major release. These breaking changes are not critical and we can add the new function as get_dt_container_cls before changing or removing the old get_container_cls. Make a new bugfix release 2.5.1 and perhaps remove 2.5.0 from PyPI and conda-forge.

See also #580 and AllenInstitute/AllenSDK#2098 .

Checklist

  • Did you update CHANGELOG.md with your changes?
  • Have you checked our Contributing document?
  • Have you ensured the PR clearly describes the problem and the solution?
  • Is your contribution compliant with our coding style? This can be checked running flake8 from the source directory.
  • Have you checked to ensure that there aren't other open Pull Requests for the same change?
  • Have you included the relevant issue number using "Fix #XXX" notation where XXX is the issue number? By including "Fix #XXX" you allow GitHub to close issue #XXX when the PR is merged.

@rly rly requested review from oruebel and ajtritt April 23, 2021 07:55
@rly rly merged commit 73d9336 into dev Apr 23, 2021
@rly rly deleted the fix/revert_get_container_cls branch April 23, 2021 16:04
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