Skip to content

Enabling attribute tests for reading data from groups #104

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
15 changes: 10 additions & 5 deletions test/tZarrAttributes.m
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ function verifyArrayAttributeInfo(testcase)

function verifyAttrOverwrite(testcase)
% Verify attribute value after overwrite.
%testcase.assumeTrue(false,'Filtered until the attributes display is fixed.');
expAttrStr = ["new","attribute","value"];
zarrwriteatt(testcase.ArrPathWrite,'attr1',expAttrStr);
expAttrDbl = 10;
Expand All @@ -53,11 +52,17 @@ function verifyAttrOverwrite(testcase)
end

function verifyGroupAttributeInfo(testcase)
% Write attribute info using zarrwriteatt function to a group.
testcase.assumeTrue(false,'Filtered until Issue-35 is fixed.');
% Verify group attribute info.
Copy link
Member

Choose a reason for hiding this comment

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

Do we already have a test for zarrwriteatt for groups?

grpPath = 'grp_v2/';
grpInfo = zarrinfo(grpPath);

actAttr.attr1 = grpInfo.group_description;
actAttr.attr2 = grpInfo.group_level;
Comment on lines +59 to +60
Copy link
Member

Choose a reason for hiding this comment

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

Instead of creating an artificial "actual" struct value from two fields of the actual output of zarrinfo, I think I would have preferred a separate verifyEqual for each field. In addition to being conceptually nicer (IMHO), I think it would also make it easier to see which attribute has an issue if the test fails.


expAttr.attr1 = 'This is a sample Zarr group';
Copy link
Member

Choose a reason for hiding this comment

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

hang on, in the file itself the attribute names are "group_description" and "group_level", but we are using attr1 and attr2 for the field names? A bit confusing..

Would avoid this issue if checking each field separately which would be my preference (see other comment) :)

expAttr.attr2 = 1;

% Unable to read attribute data from a group/array created
% using Python.
testcase.verifyEqual(actAttr,expAttr,'Failed to verify group info.');
end

function verifyZarrV3WriteError(testcase)
Expand Down