Add model and serial_number parameters to mock_Device - #2238
Draft
rly wants to merge 2 commits into
Draft
Conversation
Device links to a DeviceModel through its `model` field, and carries a `serial_number`. `mock_Device` exposed neither, so it could only build a Device whose only metadata came from the deprecated `manufacturer` field. Adding a `model` to an NWBFile requires the DeviceModel to be in that same file, since `Device.model` is a link. `mock_Device` therefore adds the DeviceModel to the NWBFile when it is not already present, which keeps the one-line mock call writable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #2238 +/- ##
=======================================
Coverage 96.04% 96.04%
=======================================
Files 30 30
Lines 3010 3010
Branches 437 437
=======================================
Hits 2891 2891
Misses 67 67
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Follow-up to the review of #2232, which surfaced
mock_Deviceas the other half of theDevice/DeviceModelgap.Devicelinks to aDeviceModelthrough itsmodelfield, and carries aserial_number.mock_Deviceexposed neither. The only device metadata it could set wasmanufacturer, whichDevicedeprecates in favor ofDeviceModel.manufacturerand warns on. Somock_Devicehad no way to produce aDevicein the shape the schema now recommends.This PR adds
modelandserial_numberparameters and passes them through.The
modelcase needs one piece of care:Device.modelis a link, so theDeviceModelhas to be in the sameNWBFileor the write raises an orphan-container error.mock_Deviceadds theDeviceModelto theNWBFilewhen it is not already there, keeping the one-line call writable:Passing a
DeviceModelthat is already in the file (themock_DeviceModel(nwbfile=nwbfile)case) links it without a second add. Passing a differentDeviceModelwhose name is already taken raisesValueErrorfromadd_device_modelrather than silently leaving a dangling link.manufactureris left in place. It is deprecated onDeviceand already warns fromDevice.__init__, butpynwb.testing.mockis public API and removing the parameter would break downstream callers. Happy to deprecate it here on its own schedule if you would prefer.Notes for the reviewer
serial_numberis slightly beyond the reviewed gap. It is the only other non-deprecatedDevicefield the mock could not set, so including it makesmock_Devicecomplete rather than inviting an immediate follow-up. Say the word and I will drop it.### Addedin4.1.1 (Unreleased). New parameters arguably belong in a minor release, so move it to a4.2.0section if that fits the release plan better.dev, not off Fix mock_DeviceModel defaulting manufacturer to None #2232. The new tests passmanufacturertomock_DeviceModelexplicitly, so they do not depend on that PR landing first, and there is no conflict between the two branches.How to test the behavior?
Four unit tests were added to
tests/unit/test_mock.py, covering the link, the add-to-file behavior, the already-present case, the name clash, and a round-trip write.Run locally on Python 3.14, HDMF 6.x:
pytest tests/unitpytest tests/integration/hdf5ruff check .codespelldocs/source/tutorials/output)Checklist
ruff check . && codespellfrom the source directory.No issue filed; this came out of the #2232 review discussion.
🤖 Generated with Claude Code