-
Notifications
You must be signed in to change notification settings - Fork 3
Test cleanup: fixtures, coverage, and stub removal #132
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
base: main
Are you sure you want to change the base?
Changes from all commits
3fd91c4
410df3e
8ad3b9c
98018b6
dcf8b7a
5a8c3bb
36a142a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # CLAUDE.md | ||
|
|
||
| This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. | ||
|
|
||
| ## Overview | ||
|
|
||
| MATLAB interface for reading and writing Zarr v2 arrays and metadata, from both local storage and Amazon S3. The MATLAB layer delegates the actual Zarr I/O to Google's [tensorstore](https://github.com/google/tensorstore) Python library, which it calls through MATLAB's `py.` Python bridge. | ||
|
|
||
| ## Architecture | ||
|
|
||
| The codebase is a three-language stack. Data and type information flow across all three layers, so a change to the data path usually touches each: | ||
|
|
||
| 1. **User-facing MATLAB functions** (`zarrread.m`, `zarrwrite.m`, `zarrcreate.m`, `zarrinfo.m`, `zarrwriteatt.m`) — thin wrappers that do `arguments`-block input validation and construct a `Zarr` object. These are the documented public API. | ||
|
|
||
| 2. **`Zarr.m`** — the central gateway class (`classdef Zarr < handle`). It owns the connection between MATLAB and Python: bootstrapping the Python module path (`pySetup`/`ZarrPy`), building the tensorstore KVStore schema (local `file` driver vs. S3 `s3` driver), resolving/creating paths and Zarr groups, validating partial-read parameters, and converting between MATLAB and numpy arrays. Most non-trivial logic lives here as static helper methods. | ||
|
|
||
| 3. **`PythonModule/ZarrPy.py`** — a small wrapper over tensorstore. Exposes `createKVStore`, `createZarr`, `writeZarr`, `readZarr`. This is the only code that talks to tensorstore directly. `Zarr.m` imports it via `py.importlib.import_module('ZarrPy')` after inserting `PythonModule/` onto `py.sys.path`. | ||
|
|
||
| ### Key cross-cutting concerns | ||
|
|
||
| - **Datatype mapping** (`ZarrDatatype.m`): a single class holds three parallel arrays mapping MATLAB types ↔ tensorstore types ↔ Zarr dtype strings (e.g. `"double"` ↔ `"float64"` ↔ `"<f8"`). Construct via the static `fromMATLABType` / `fromTensorstoreType` / `fromZarrType` methods, never the private constructor. Any new supported datatype must be added to all three arrays in lockstep. | ||
|
|
||
| - **Index convention conversion**: MATLAB is 1-based and uses *count*; tensorstore is 0-based and uses *end index* (exclusive). The translation happens in `Zarr.read` (`start = start - 1`, `endInds = start + stride.*count`). Partial-read validation (Start/Stride/Count bounds, scalar-into-vector indexing) is in `Zarr.processPartialReadParams`. | ||
|
|
||
| - **Local vs. remote (S3)**: `obj.isRemote` is detected from an IRI prefix on the path. S3 URLs/URIs in six different formats are parsed into bucket + object path by `Zarr.extractS3BucketNameAndPath`. Some validity checks (e.g. `isZarrArray`) are skipped for `http`-style remote paths because they would fail even on valid arrays. | ||
|
|
||
| - **Zarr metadata files**: `.zarray` marks an array, `.zgroup` marks a group, `.zattrs` holds user-defined attributes (all Zarr v2, read/written as JSON). `zarr.json` is the Zarr v3 metadata file — it is detected by `zarrinfo` but writing v3 is not supported. `zarrinfo.m` reads these JSON files directly in MATLAB (not via Python); creating group hierarchies writes `.zgroup` files directly too. | ||
|
|
||
| ## Commands | ||
|
|
||
| There is no build step — it's interpreted MATLAB plus a Python module on the path. | ||
|
|
||
| **Run the full test suite** (from the `test/` directory, since tests resolve data paths relative to `pwd`): | ||
| ```matlab | ||
| cd test | ||
| results = runtests('IncludeSubfolders', true) | ||
| ``` | ||
|
|
||
| **Run a single test class or method:** | ||
| ```matlab | ||
| cd test | ||
| runtests('tZarrRead') % one class | ||
| runtests('tZarrRead/verifyPartialArrayData') % one method | ||
| ``` | ||
|
|
||
| CI (`.github/workflows/test_setup.yml`) runs `matlab-actions/run-tests` with `select-by-folder: 'test'` across Ubuntu/Windows/macOS and MATLAB R2024a + latest. | ||
|
|
||
| ## Setup requirements | ||
|
|
||
| - MATLAB R2024a or newer. Add the repo root to the MATLAB path (`addpath`). | ||
| - Python 3.10+ configured for MATLAB (`pyenv`), with `numpy` and `tensorstore` installed (see `PythonModule/requirements.txt`; CI pins `tensorstore==0.1.71`, the minimum supported version). | ||
|
|
||
| When iterating on `ZarrPy.py`, MATLAB caches the imported module. Reload with `Zarr.pyReloadInProcess()` (after `clear classes`) for in-process Python, or `terminate(pyenv)` for out-of-process. | ||
|
|
||
| ## Tests | ||
|
|
||
| xUnit-style classes (`matlab.unittest.TestCase`) named `t<Feature>.m` in `test/`. They inherit shared fixtures from `SharedZarrTestSetup.m`, which adds the parent source folder to the path and copies `test/dataFiles/` into a `WorkingFolderFixture` so write tests don't pollute the repo. Read-test fixtures live in `test/dataFiles/grp_v2` (and `grp_v3`); expected results are stored in `expZarrArrData.mat` / `expZarrArrInfo.mat`. | ||
|
|
||
| ## Conventions | ||
|
|
||
| - All error messages use `error("MATLAB:<area>:<id>", ...)` identifiers — match the existing namespacing when adding new ones. | ||
| - Function help text is the block comment directly under the signature; keep it current since the README points users to `help <function>`. | ||
| - Spelling is checked in CI by codespell (`.codespellrc`); add false positives to `ignore-words-list`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,5 @@ coverage: | |
| target: 85% | ||
| threshold: 5% | ||
| ignore: | ||
| - "test/*" | ||
| - "test/*" | ||
| - "tools/*" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,104 @@ function verifyReload(testcase) | |
|
|
||
| end | ||
|
|
||
|
|
||
| function verifyIsZarrArrayAndGroup(testcase) | ||
| % Verify that isZarrArray and isZarrGroup correctly identify a | ||
| % Zarr array (has .zarray) versus a Zarr group (has .zgroup). | ||
| % SharedZarrTestSetup copies the *contents* of dataFiles into | ||
| % the working folder, so fixtures live at grp_v2/... directly. | ||
| arrPath = "grp_v2/arr_v2"; | ||
| grpPath = "grp_v2"; | ||
|
|
||
| testcase.verifyTrue(Zarr.isZarrArray(arrPath),... | ||
| "Expected an array path to be a Zarr array."); | ||
| testcase.verifyFalse(Zarr.isZarrArray(grpPath),... | ||
| "Did not expect a group path to be a Zarr array."); | ||
|
|
||
| testcase.verifyTrue(Zarr.isZarrGroup(grpPath),... | ||
| "Expected a group path to be a Zarr group."); | ||
| testcase.verifyFalse(Zarr.isZarrGroup(arrPath),... | ||
| "Did not expect an array path to be a Zarr group."); | ||
| end | ||
|
|
||
| function verifyDatatypeRoundTrip(testcase) | ||
| % Verify that ZarrDatatype maps consistently across MATLAB, | ||
| % Tensorstore, and Zarr type names, regardless of which static | ||
| % constructor is used to create it. | ||
| mlType = "double"; | ||
| tsType = "float64"; | ||
| zType = "<f8"; | ||
|
|
||
| fromML = ZarrDatatype.fromMATLABType(mlType); | ||
| fromTS = ZarrDatatype.fromTensorstoreType(tsType); | ||
| fromZarr = ZarrDatatype.fromZarrType(zType); | ||
|
|
||
| for dt = [fromML, fromTS, fromZarr] | ||
| testcase.verifyEqual(dt.MATLABType, mlType); | ||
| testcase.verifyEqual(dt.TensorstoreType, tsType); | ||
| testcase.verifyEqual(dt.ZarrType, zType); | ||
| end | ||
| end | ||
|
|
||
| function verifyInvalidTensorstoreType(testcase) | ||
| % Verify error when an unsupported Tensorstore type name is used. | ||
| testcase.verifyError(... | ||
| @()ZarrDatatype.fromTensorstoreType("not_a_type"),... | ||
| "MATLAB:validators:mustBeMember"); | ||
| end | ||
|
|
||
| function verifyCreateGroupMakesFolder(testcase) | ||
| % Verify that createGroup creates the directory when it does not | ||
| % already exist, and writes a valid .zgroup file into it. | ||
| groupPath = fullfile(pwd, "brandNewGroup"); | ||
| testcase.verifyFalse(isfolder(groupPath),... | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be removed. Claude likes to add too much of this verification stuff, which is redundant. |
||
| "Group folder should not exist before createGroup."); | ||
|
|
||
| Zarr.createGroup(groupPath); | ||
|
|
||
| testcase.verifyTrue(isfolder(groupPath),... | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this needed if you are already checking for group and you already have verification for .zgroup file? looks redundant to me, but upto you. |
||
| "createGroup should have created the folder."); | ||
| testcase.verifyTrue(isfile(fullfile(groupPath, ".zgroup")),... | ||
| "createGroup should have written a .zgroup file."); | ||
| testcase.verifyEqual(zarrinfo(groupPath).node_type, 'group',... | ||
| "createGroup should produce a valid Zarr group."); | ||
| end | ||
|
|
||
| function verifyCreateGroupOpenFailure(testcase) | ||
| % Verify error when the .zgroup file cannot be opened for | ||
| % writing. Everything lives inside an isolated temporary folder | ||
| % fixture, so no real data is modified. | ||
| % | ||
| % We make the existing .zgroup *file* read-only rather than its | ||
| % folder: a read-only folder does not prevent file creation on | ||
| % Windows (the directory read-only attribute is ignored there), | ||
| % whereas a read-only file is honored on both Windows and Unix. | ||
| import matlab.unittest.fixtures.TemporaryFolderFixture | ||
| tempFixture = testcase.applyFixture(TemporaryFolderFixture); | ||
|
|
||
| groupPath = fullfile(tempFixture.Folder, "readOnlyGroup"); | ||
| Zarr.createGroup(groupPath); % writes .zgroup | ||
| zgroupFile = fullfile(groupPath, ".zgroup"); | ||
|
|
||
| fileattrib(zgroupFile, '-w'); | ||
| % Restore write permission before the fixture is torn down so its | ||
| % contents can be removed (runs before the fixture's rmdir). | ||
| testcase.addTeardown(@()fileattrib(zgroupFile, '+w')); | ||
|
|
||
| testcase.verifyError(@()Zarr.createGroup(groupPath),... | ||
| "MATLAB:Zarr:fileOpenFailure"); | ||
| end | ||
|
|
||
| function verifyWriteScalarShapedArray(testcase) | ||
| % Verify writing to an array whose stored shape is a true scalar | ||
| % (shape [1]). This exercises the isscalar(info.shape) branch of | ||
| % Zarr.write, which zarrcreate cannot produce on its own because | ||
| % it expands scalar sizes to [1 N]. | ||
| scalarPath = "grp_v2/scalarData"; | ||
|
|
||
| zarrwrite(scalarPath, 42); | ||
| testcase.verifyEqual(zarrread(scalarPath), 42,... | ||
| "Failed to write/read a scalar-shaped Zarr array."); | ||
| end | ||
|
|
||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you create it TemporaryFolder?