Skip to content

Avoid dereferencing external group links during in-memory copies - #275

Open
CAOShurong wants to merge 1 commit into
nexpy:mainfrom
CAOShurong:codex/205-copy-linked-groups-in-memory
Open

CAOShurong wants to merge 1 commit into
nexpy:mainfrom
CAOShurong:codex/205-copy-linked-groups-in-memory

Conversation

@CAOShurong

Copy link
Copy Markdown
Contributor

Problem

Copying a group that contains a relative external group link into an unsaved
NXroot still fails on current main. The assignment deep-copies the group,
then calls update(). Its in-memory walk() follows the linked group, so the
external filename is resolved relative to the process working directory before
the destination has a filename. A valid external.nxs beside the source file
can consequently raise NeXusError: .../external.nxs does not exist.

Related to #205. This is a reproduced residual copy failure, not a claim to
reproduce or fix every part of that issue's original h5py traceback. The link
preservation from #234 and relative-filename behavior from #259 are retained.

Change

Limit the in-memory update to nodes owned by the copied tree, skipping NXlink
targets. Ordinary lazy fields are still materialized, including fields nested
inside ordinary groups. Public walk(), saved-file updates, link targets and
stored relative/absolute filenames are unchanged.

Add ten regressions covering direct/nested external group links, relative/
absolute filenames, unsaved/file-backed destinations, and ordinary lazy fields
that remain readable after the original file is closed and renamed. The link
cases verify the stored HDF5 external link and reopened data/units.

Verification

Executed on Windows with Python 3.13.1, NumPy 2.5.3 and h5py 3.16.0, comparing
base 83c50ea6 to this committed head. Pytest source runs used an additional
wrapper that asserts the Git head, import path and exact committed source bytes
before invoking pytest; installed runs verified a separate noneditable package.

  • Baseline full suite: 137 passed, twice. A byte-verified installed baseline API
    probe reproduces the relative-group copy failure (1 failed / 9 passed).
  • Added regressions on unchanged product code: 4 failed / 6 passed. Besides the
    relative-path failures, absolute-link controls detect unwanted eager loading.
  • pytest tests/test_links.py tests/test_files.py -k test_copy_group -vv:
    10 passed repeatedly, including twice on the committed source.
  • Related link/file modules: 32 passed, repeated.
  • Full committed-source and independent installed suites: 147 passed each.
    JUnit comparison preserves all 137 old outcomes and adds ten passing cases;
    all 147 source/installed outcomes agree.
  • ruff check ., configured all-files pre-commit run --all-files, and
    git diff --check: passed.
  • Real-SCM sdist, wheel built from the extracted sdist, independent noneditable
    installation and pip check: passed. All 353 tracked source/sdist files and
    328 package files in the wheel/install match the committed bytes.
  • Installed real-HDF5 API probe: all ten cases passed twice (copy, save/reopen,
    signal/axes/units, readonly rejection, unchanged original-file hashes).
  • Installed nxduplicate / nxdir: six real success/failure paths passed,
    including external-link expansion, refusal to overwrite and missing input.

The full suites retain the existing NumPy shape deprecation at unchanged
tree.py:4012 (six baseline warnings, eight with the two added lazy-field cases).
Initial local CRLF/provenance, test-harness and lint failures were retained and
corrected; they are not counted as passing runs. The SCM build is a development
version, not a new upstream release. No configured type-checker/docs-build gate
was found. NeXpy GUI, production datasets, physical acquisition and other local
OS/Python combinations were not tested; upstream CI/acceptance is still pending.

AI assistance

Code, tests and this description were generated/edited with Codex assistance.
The stated verification was actually executed automatically. No personal human
review, maintainer approval or downstream adoption is claimed.

@rayosborn

Copy link
Copy Markdown
Contributor

Thank you for your work on this. My own Claude Code checks confirm that the suggested changes are valid and don't have unwanted side effects. I am just about to release v2.1.0 and would rather not delay that without running further checks, so I hope to merge this later this week and the release v2.1.1.

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