Skip to content

Drop pynwb < 3.1 compatibility code from pynwb_utils - #1939

Merged
CodyCBakerPhD merged 1 commit into
masterfrom
claude/pynwb-utils-drop-legacy-pynwb
Oct 6, 2026
Merged

CodyCBakerPhD merged 1 commit into
masterfrom
claude/pynwb-utils-drop-legacy-pynwb

Conversation

@CodyCBakerPhD

@CodyCBakerPhD CodyCBakerPhD commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Yet another split out of #1933 to clean up some dead code

The declared floor in the pyproject.toml is pynwb >= 3.1.0 (and nwbinspector >= 0.7.0 requires pynwb >= 3.1 anyway), which makes several version guards unreachable:

  • validate(): only the pynwb.validate(path=...) branch (pynwb >= 3.0) can run; the paths=[...] tuple-returning form (2.2 to 2.x) and the io= fallback for even older releases are gone.
  • copy_nwb_file(): cache_spec has been accepted by export() since pynwb 2.8.2, so pass it unconditionally.
  • _get_external_images(): ExternalImage exists in every supported pynwb, so import it at module level instead of guarding an ImportError.

The now-unused packaging.version.Version import is removed as well. The NWB schema version checks (e.g. the < 2.1.0 error filter) are about the file being validated, not about pynwb, and are unchanged.

Claude-Session: https://claude.ai/code/session_01HFhMPs6vF5zyeHDvPTvRcA

@CodyCBakerPhD CodyCBakerPhD self-assigned this Sep 27, 2026
@codecov

codecov Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.41%. Comparing base (563f2d7) to head (91aaca7).

Files with missing lines Patch % Lines
dandi/pynwb_utils.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1939   +/-   ##
=======================================
  Coverage   78.40%   78.41%           
=======================================
  Files          92       92           
  Lines       14142    14130   -12     
=======================================
- Hits        11088    11080    -8     
+ Misses       3054     3050    -4     
Flag Coverage Δ
unittests 78.41% <50.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@CodyCBakerPhD
CodyCBakerPhD added this pull request to stack #1940 September 27, 2026 19:29
@CodyCBakerPhD CodyCBakerPhD added the internal Changes only affect the internal API label Sep 27, 2026
@CodyCBakerPhD

Copy link
Copy Markdown
Contributor Author

Note: code is technically covered by the py3-lowest tox environment test that doesn't upload to codecov (could fix that after this stack, seems minor)

@CodyCBakerPhD
CodyCBakerPhD removed this pull request from stack #1940 September 27, 2026 20:17
@CodyCBakerPhD
CodyCBakerPhD added this pull request to stack #1942 September 27, 2026 20:17
@yarikoptic

Copy link
Copy Markdown
Member

great , thanks!

@yarikoptic

Copy link
Copy Markdown
Member

I would have merged it but it seems part of the stack.... yet to learn the process ;-)

@yarikoptic
yarikoptic force-pushed the claude/pynwb-utils-drop-legacy-pynwb branch from 023745f to dab93f5 Compare September 28, 2026 14:38
@CodyCBakerPhD

Copy link
Copy Markdown
Contributor Author

I would have merged it but it seems part of the stack.... yet to learn the process ;-)

Yeah stacks can go either way - if a long series of changes that are intended to go in all at once as 'some big feature' then start from top then go down

For this I strongly recommend just starting from the bottom and working the way up, many of those are nearly-standalone improvements but still cleaner for the top to not deal with them all split apart

Base automatically changed from claude/validate-broken-symlink-path to master September 30, 2026 00:59
@yarikoptic
yarikoptic force-pushed the claude/pynwb-utils-drop-legacy-pynwb branch from dab93f5 to 16cdedd Compare September 30, 2026 00:59
@CodyCBakerPhD

Copy link
Copy Markdown
Contributor Author

@yarikoptic This one is next, once CI is happy

The declared floor is pynwb >= 3.1.0 (and nwbinspector >= 0.7.0 requires
pynwb >= 3.1 anyway), which makes several version guards unreachable:

- validate(): only the `pynwb.validate(path=...)` branch (pynwb >= 3.0) can
  run; the `paths=[...]` tuple-returning form (2.2 to 2.x) and the
  `io=` fallback for even older releases are gone.
- copy_nwb_file(): `cache_spec` has been accepted by `export()` since
  pynwb 2.8.2, so pass it unconditionally.
- _get_external_images(): `ExternalImage` exists in every supported
  pynwb, so import it at module level instead of guarding an ImportError.

The now-unused `packaging.version.Version` import is removed as well.
The NWB *schema* version checks (e.g. the < 2.1.0 error filter) are about
the file being validated, not about pynwb, and are unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFhMPs6vF5zyeHDvPTvRcA
@CodyCBakerPhD
CodyCBakerPhD force-pushed the claude/pynwb-utils-drop-legacy-pynwb branch from 16cdedd to 91aaca7 Compare October 6, 2026 18:01
@CodyCBakerPhD
CodyCBakerPhD merged commit 8b6a1fc into master Oct 6, 2026
40 of 41 checks passed
@CodyCBakerPhD
CodyCBakerPhD deleted the claude/pynwb-utils-drop-legacy-pynwb branch October 6, 2026 18:32
CodyCBakerPhD pushed a commit that referenced this pull request Oct 6, 2026
…hamilton-ast647

The base was rebased onto master (with #1937 and #1939 squash-merged)
and now uses fscacher >= 0.5.0's memoize_path(custom_fingerprint=...).
Resolve accordingly:

- Drop memoize_source and its tests: get_metadata, get_neurodata_types,
  nwb_has_external_links and _validate now use
  memoize_path(custom_fingerprint=readable_fingerprint).
- Drop ExistingPath in favor of LinkAwarePath(lexists=True) from #1937.
- Keep the streaming support (_validate reading from a Readable) and
  its test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qn5WBSiQgoZoL4fytF6nEr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Changes only affect the internal API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants