Skip to content

Make zarrcreate create intermediate zarr groups and work with relative paths #91

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 16, 2025
Merged

Make zarrcreate create intermediate zarr groups and work with relative paths #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca47715
Make zarrcreate create intermediate zgroups and work with .. relative…
May 13, 2025
8cb87ad
Fix comment typo
krisfed May 14, 2025
ea86b5e
Fix another comment typo
krisfed May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/test_setup.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ on:
- branch-bug
- update-test-branch
- test-branch7
- test-branch8

jobs:
test:
Expand Down Expand Up @@ -45,4 +46,4 @@ jobs:
with:
token: ${{secrets.CODECOV_TOKEN}}
files: ./target/site/cobertura/coverage.xml
name: codecov-umbrella.xml
name: codecov-umbrella.xml
137 changes: 135 additions & 2 deletions Zarr.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
% Copyright 2025 The MathWorks, Inc.

properties(GetAccess = public, SetAccess = protected)
Path
Path (1,1) string
ChunkSize
DsetSize
FillValue
Expand Down Expand Up @@ -56,6 +56,122 @@
py.importlib.reload(zarrModule);
end
end

function isZarray = isZarrArray(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these methods also be made protected to limit access to only the class/object of the class?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point.. I was thinking at some point we might want to expose some of them (esp createGroup()) through a separate function or move them elsewhere all together. If no big flag, I think we can keep it as is for now?

% Given a path, determine if it is a Zarr array

isZarray = isfile(fullfile(path, '.zarray'));
end

function isZgroup = isZarrGroup(path)
% Given a path, determine if it is a Zarr group

isZgroup = isfile(fullfile(path, '.zgroup'));

Check warning on line 69 in Zarr.m

View check run for this annotation

Codecov / codecov/patch

Zarr.m#L69 

Added line #L69 was not covered by tests
end

function resolvedPath = getFullPath(path)
% Given a path, resolves it to a full path. The trailing
% directories do not have to exist.

arguments (Input)
path (1,1) string
end

if path == ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand this. An empty string must not be allowed, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this function works recursively - it checks if a given path is resolvable to an absolute path, and if not, then it calls itself on the parent folder (until it finds a resolvable absolute path). But if you start with a relative path (say "myzarr/A/B") where none of the folders exist yet, none of "myzarr/A/B", "myzarr/A", or "myzarr" will resolve to an absolute path, so the recursion will end up with a parent folder that is "". At that point, we know that the intention is that "myzarr/A/B" folders will be created at the current location (pwd), so the full path should be pwd + "myzarr/A/B"

resolvedPath = pwd;
return
end

resolvedPath = matlab.io.internal.filesystem.resolvePath(path).ResolvedPath;

if resolvedPath == ""
% If the given path does not exist, it is likely due to
% trailing directories not existing yet. Resolve parent
% directory's path, and append child directory.

[pathToParentFolder, child, ext] = fileparts(path);

resolvedParentPath = Zarr.getFullPath(pathToParentFolder);
resolvedPath = fullfile(resolvedParentPath, child+ext);
end
end

function existingParent = getExistingParentFolder(path)
% Given a full path where some trailing directories might not yet
% exist, determine the longest prefix path that does exist

arguments (Input)
path (1,1) string
end

if isfolder(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check if this works for S3?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I tried a simple workflow like this, but let me know if there is something specific I should check:

>> s3ParentFolder = "s3://<...reducted...>/"
>> zarrcreate(s3ParentFolder + "myzarr.zarr/A/B/C", [100,100])
>> d = zarrread(s3ParentFolder + "myzarr.zarr/A/B/C");
>> info = zarrinfo(s3ParentFolder + "myzarr.zarr")

info = 

  [struct](matlab:helpPopup('struct')) with fields:

    zarr_format: '2'
      node_type: 'group'

>> info = zarrinfo(s3ParentFolder + "myzarr.zarr/A")

info = 

  [struct](matlab:helpPopup('struct')) with fields:

    zarr_format: '2'
      node_type: 'group'
 
>> info = zarrinfo(s3ParentFolder + "myzarr.zarr/A/B")

info = 

  [struct](matlab:helpPopup('struct')) with fields:

    zarr_format: '2'
      node_type: 'group'

>> info = zarrinfo(s3ParentFolder + "myzarr.zarr/A/B/C")

info = 

  [struct](matlab:helpPopup('struct')) with fields:

                 chunks: [2×1 double]
             compressor: []
    dimension_separator: '.'
                  dtype: '<f8'
             fill_value: []
                filters: []
                  order: 'C'
                  shape: [2×1 double]
            zarr_format: 2
              node_type: 'array'
>> rmdir(s3ParentFolder + "myzarr.zarr", 's')

I have also tried this method separately too:

>> existParent = Zarr.getExistingParentFolder(s3ParentFolder + "myzarr.zarr/A/B/C");
>> existParent + "/" == s3ParentFolder

ans =

  [logical](matlab:helpPopup('logical'))

   1

>>

% if the full path exist, we are done
existingParent = path;
return
end

% See if the parent path exist. Continue recursing until an
% exisiting parent path is found
[pathToParentFolder, ~, ~] = fileparts(path);
existingParent = Zarr.getExistingParentFolder(pathToParentFolder);

end

function createGroup(pathToGroup)
% Create a Zarr group including creating the directory (if
% needed) and the .zgroup file. Assumes the parent directory
% exists

if ~isfolder(pathToGroup)
mkdir(pathToGroup)

Check warning on line 126 in Zarr.m

View check run for this annotation

Codecov / codecov/patch

Zarr.m#L126 

Added line #L126 was not covered by tests
end

% Currently we support only Zarr v2
groupJSON = jsonencode(struct("zarr_format", "2"));

% Write .zgroup file
groupFile = fullfile(pathToGroup, ".zgroup");
fid = fopen(groupFile, 'w');
if fid == -1
error("MATLAB:Zarr:fileOpenFailure",...
"Could not open file ""%s"" for writing.",groupFile);

Check warning on line 137 in Zarr.m

View check run for this annotation

Codecov / codecov/patch

Zarr.m#L136-L137 

Added lines #L136 - L137 were not covered by tests
end
closeFile = onCleanup(@() fclose(fid));

fwrite(fid, groupJSON, 'char');
end

function makeZarrGroups(existingParentPath, newGroupsPath)
% Create a hierarchy of nested Zarr groups for all directories
% in newGroupsPath. For example, if existingParentPath is
% "/Users/jsmith/Documents" and newGroupsPath is
% "myfile.zarr/A/B", the following directories will be made
% into Zgroups:
% /Users/jsmith/Documents/myfile.zarr/
% /Users/jsmith/Documents/myfile.zarr/A
% /Users/jsmith/Documents/myfile.zarr/A/B
%
% The existingParentPath and newGroupsPath should combine to
% create an absolute path to the most nested zarr group to be
% created

arguments (Input)
existingParentPath (1,1) string
newGroupsPath (1,1) string
end

newGroups = split(newGroupsPath, filesep);

for group = newGroups'
if group == ""
continue
end
pathToNewGroup = fullfile(existingParentPath, group);
Zarr.createGroup(pathToNewGroup);
existingParentPath = pathToNewGroup;
end

end
end

methods
Expand Down Expand Up @@ -86,6 +202,8 @@
obj.KVStoreSchema = py.dict(RemoteStoreSchema);

else % Local file
% use full path
obj.Path = Zarr.getFullPath(path);
FileStoreSchema = dictionary(["driver", "path"], ["file", obj.Path]);
obj.KVStoreSchema = py.dict(FileStoreSchema);
end
Expand All @@ -100,7 +218,7 @@
% Zarr files in the s3:// syntax (for now) because https S3
% links will fail this check even if they are valid.
if ~startsWith(obj.Path, 'http')
if ~isfile(fullfile(obj.Path, '.zarray'))
if ~Zarr.isZarrArray(obj.Path)
error("MATLAB:Zarr:invalidZarrObject",...
"Invalid file path. File path must refer to a valid Zarr array.");
end
Expand Down Expand Up @@ -144,12 +262,27 @@
obj.FillValue = cast(fillvalue, obj.MatlabDatatype);
end

% see how much of the provided path exists already
existingParentPath = Zarr.getExistingParentFolder(obj.Path);

% The Python function returns the Tensorstore schema, but we
% do not use it for anything at the moment.
obj.TensorstoreSchema = py.ZarrPy.createZarr(obj.KVStoreSchema, py.numpy.array(obj.DsetSize),...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, you are identifying the folders to be created before createZarr and after the creation, you are populating the groups with .zgroup file. Smart!!
Neat use of recursion!!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern would be to do manual testing on S3 buckets (in case you havent done already)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! :)
It was the best way I could figure out without having to undo folder creation error cases... Still a bit clunky that we have to do it manually. Based on Zarr spec, I almost want to say it is a bug in tensorstore that it doesn't create intermediate zarr groups

py.numpy.array(obj.ChunkSize), obj.TensorstoreDatatype, ...
obj.ZarrDatatype, obj.Compression, obj.FillValue);
%py.ZarrPy.temp(py.numpy.array([1, 1]), py.numpy.array([2, 2]))

% if new directories were created as part of creating a
% Zarr array, we need to make them into Zarr groups.
newDirs = extractAfter(obj.Path, existingParentPath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if newDirs is empty?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the newGroups will also be empty and we will skip creating Zarr groups because of the if statement that checks for empty newGroups.

>> [newGroups, ~,~] = fileparts("")

newGroups = 

    ""

% the last directory is a Zarr array, ones before should be
% Zarr groups
[newGroups, ~,~] = fileparts(newDirs);
if newGroups ~= ""
Zarr.makeZarrGroups(existingParentPath, newGroups);
end


end

function write(obj, data)
Expand Down
23 changes: 23 additions & 0 deletions test/tZarrCreate.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,29 @@
% Copyright 2025 The MathWorks, Inc.

methods(Test)

function createIntermediateZgroups(testcase)
% Verify that zarrcreate creates zarr groups when given a
% nested path

arrayPath = fullfile(testcase.ArrPathWrite, "A", "B");
zarrcreate(arrayPath, testcase.ArrSize);
[groupPath, ~, ~] = fileparts(arrayPath);

testcase.verifyTrue(isfile(fullfile(groupPath, ".zgroup")),...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we doing this check for both A and B?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in this case B will be a Zarr array (the last element of the path we are passing into zarrcreate).
Not sure if we need a test for a more nested case like fullfile(testcase.ArrPathWrite, "A", "B", "C")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you check if this behaves as expected for a Zarr file hosted on an S3 bucket?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a basic check (see comment here: https://github.com/mathworks/MATLAB-support-for-Zarr-files/pull/91/files#r2089009441 ). Let me know if anything else needs to be tried! Wish we had an easier way to run automated tests with S3 locations

".zgroup file was not created")

grpInfo = zarrinfo(groupPath);
expFormat = '2';
expType = 'group';

testcase.verifyEqual(grpInfo.zarr_format, expFormat,...
"Unexpected Zarr group format");
testcase.verifyEqual(grpInfo.node_type, expType,...
"Unexpected Zarr group node type");

end

function invalidFilePath(testcase)
% Verify error when an invalid file path is used as an input to
% zarrcreate function.
Expand Down
4 changes: 2 additions & 2 deletions test/tZarrInfo.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ function verifyGroupInfoV3(testcase)
end

function missingZgroupFile(testcase)
% Verify error when using zarrinfo function on a group not
% Verify error when using zarrinfo function on a directory not
% containing .zgroup file.
import matlab.unittest.fixtures.WorkingFolderFixture;
testcase.applyFixture(WorkingFolderFixture);

zarrcreate('prt_grp_write/arr1',[10 10]);
mkdir('prt_grp_write/arr1');
grpPath = 'prt_grp_write/';
errID = 'MATLAB:zarrinfo:invalidZarrObject';
testcase.verifyError(@()zarrinfo(grpPath),errID);
Expand Down
6 changes: 3 additions & 3 deletions zarrcreate.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ function zarrcreate(filepath, datasize, options)
% ZARRCREATE(FILEPATH, DATASIZE, Name=Value) creates a Zarr
% array at the path specified by FILEPATH and of the dimensions specified
% by DATASIZE.
% If FILEPATH is a full path name, the function creates all
% intermediate groups that do not already exist. If FILEPATH exists
% already, the contents are overwritten.
% If FILEPATH is a full path name, the function creates all intermediate
% directories that do not already exist and makes them into Zarr groups. If
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure about making this change in the doc... Let me know what you think! Does this also need to be in-sync with readme?

% FILEPATH exists already, the contents are overwritten.
%
% Name - Value Pairs
% ------------------
Expand Down
Loading