Refactor load_namespaces to return source types, fix registration #1372
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1372 +/- ##
==========================================
- Coverage 92.90% 92.86% -0.05%
==========================================
Files 41 41
Lines 9909 9919 +10
Branches 2024 2027 +3
==========================================
+ Hits 9206 9211 +5
- Misses 431 433 +2
- Partials 272 275 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 tasks
oruebel
reviewed
Feb 8, 2026
oruebel
reviewed
Feb 8, 2026
Comment on lines
+465
to
+469
| warnings.warn( | ||
| "copy_mappers is deprecated and will be removed in a future version. " | ||
| "Use merge instead with the argument ns_catalog=False to copy only mappers without namespaces.", | ||
| DeprecationWarning, | ||
| ) |
Contributor
There was a problem hiding this comment.
Since copy_mappers is probably only used internally, I think it would be Ok to remove it with the upcoming major release. Up to you.
Contributor
Author
There was a problem hiding this comment.
I thought so too, but it was used by pynwb, and it doesn't really hurt to do this a little gradually just in case.
oruebel
reviewed
Feb 8, 2026
oruebel
previously approved these changes
Feb 8, 2026
oruebel
approved these changes
Feb 9, 2026
rly
added a commit
that referenced
this pull request
Feb 9, 2026
* Refactor load_namespaces to return source types and fix registration issue * Update test, comment * Add tests * Update tests * Update changelog * Deprecate TypeMap.copy_mappers * Update changelog * Fix RST issues * Fix RST * Don't change return type of load_namespaces * Apply suggestions from code review * Add missing tests * Refactor and clarify check when getting container cls and no ns
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
load_namespaces(from TypeMap, NamespaceCatalog, and the IO classes) to fixDynamicTableRegionwritten withhdmf_experimentalinstead ofhdmf_commonwhen using NWB extensions #1347 and improve the code structure for clarity and utility. This change will target HDMF 5.0 and needs to be rebased.TODO:
How to test the behavior?
Checklist
CHANGELOG.mdwith your changes?