Skip to content
Open
Show file tree
Hide file tree
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
10 changes: 7 additions & 3 deletions libnczarr/zcvt.c
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,13 @@ NCZ_convert1(const NCjson* jsrc, nc_type dsttype, NCbytes* buf)
break;
case NC_STRING: /* NaN might be quoted */
switch (naninftest(zcvt.strv,&naninf,&naninff)) {
case NC_NAT: abort();
case NC_FLOAT:
case NC_DOUBLE:
case NC_NAT:
// try to parse string as destination type
if(sscanf(zcvt.strv,"%lf",&d))
break;
abort();
case NC_FLOAT:
case NC_DOUBLE:
d = naninf; break;
break;
}
Expand Down
6 changes: 6 additions & 0 deletions libnczarr/zinfer.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ int NCZ_infer_zarr_format(NC_FILE_INFO_T *file) {
int stat = NC_ENOTZARR;
NCZ_FILE_INFO_T *zfile = (NCZ_FILE_INFO_T *)file->format_file_info;

int try_consolidated = NCZ_use_consolidated(zfile);
if(NC_NOERR == nczmap_exists(zfile->map, Z2METADATA)){
zfile->format.zarr = 2;
return NC_NOERR;
}

struct ZarrObjects {
const char *name;
int format;
Expand Down
4 changes: 2 additions & 2 deletions libnczarr/zmetadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ int NCZMD_get_metadata_format(NCZ_FILE_INFO_T *zfile, int *zarrformat)
return NC_NOERR;
}

int use_consolidated_metadata(NCZ_FILE_INFO_T *zfile) {
int NCZ_use_consolidated(NCZ_FILE_INFO_T *zfile) {
int use_consolidated = NCZARR_CONSOLIDATED_DEFAULT || (zfile->controls.flags & FLAG_CONSOLIDATED);
const char *e = getenv(NCZARR_CONSOLIDATED_ENV);

Expand All @@ -101,7 +101,7 @@ int use_consolidated_metadata(NCZ_FILE_INFO_T *zfile) {
int NCZMD_set_metadata_handler(NCZ_FILE_INFO_T *zfile) {
NCjson *jcsl = NULL;

int use_consolidated = use_consolidated_metadata(zfile);
int use_consolidated = NCZ_use_consolidated(zfile);
if (!use_consolidated){
nclog(NCLOGNOTE, "Not using consolidated metadata! Doing so could improve reading performance");
}
Expand Down
5 changes: 5 additions & 0 deletions libnczarr/zmetadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ extern int NCZMD_update_json_attrs(struct NCZ_FILE_INFO *zfile, const char *key,
/// @return NO_ERROR on success, error code on failure
extern int NCZMD_update_json_array(struct NCZ_FILE_INFO *zfile, const char *key, const NCjson *jarrays);

/// @brief Helper to determine if consolidated metadata should be used.
/// @param zfile
/// @return 0 is not, != 0 otherwise
extern int NCZ_use_consolidated(struct NCZ_FILE_INFO *zfile);

#if defined(__cplusplus)
}
#endif
Expand Down
20 changes: 17 additions & 3 deletions libnczarr/zsync.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ ncz_sync_var_meta(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var, int isclose)
/* Integer list defining the length of each dimension of the array.*/
/* Create the list */
NCJnew(NCJ_ARRAY,&jtmp);
if(zvar->scalar) {
if(zvar->scalar && !purezarr) {
NCJaddstring(jtmp,NCJ_INT,"1");
} else for(i=0;i<var->ndims;i++) {
snprintf(number,sizeof(number),"%llu",shape[i]);
Expand Down Expand Up @@ -390,7 +390,7 @@ ncz_sync_var_meta(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var, int isclose)
if((stat = NCJaddstring(jvar,NCJ_STRING,"chunks"))<0) {stat = NC_EINVAL; goto done;}
/* Create the list */
NCJnew(NCJ_ARRAY,&jtmp);
if(zvar->scalar) {
if(zvar->scalar && !purezarr) {
NCJaddstring(jtmp,NCJ_INT,"1"); /* one chunk of size 1 */
} else for(i=0;i<var->ndims;i++) {
size64_t len = var->chunksizes[i];
Expand Down Expand Up @@ -765,7 +765,7 @@ ncz_sync_atts(NC_FILE_INFO_T* file, NC_OBJ* container, NCindex* attlist, NCjson*
*/
NCJnew(NCJ_ARRAY,&jdimrefs);
/* Fake the scalar case */
if(var->ndims == 0)
if(var->ndims == 0 && !purezarr)
NCJaddstring(jdimrefs,NCJ_STRING,XARRAYSCALAR);
/* Walk the dimensions and capture the names */
for(i=0;i<var->ndims;i++) {
Expand Down Expand Up @@ -1562,10 +1562,24 @@ define_var1(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp, const char* varname)
/* Process the rank */
zarr_rank = NCJarraylength(jvalue);
if(zarr_rank == 0) {
// No shape, no chunks, no (xarray) dimensions => scalar
if(zvar->xarray == NULL) {
// get xarray attributes
if((stat = ncz_read_atts(file,(NC_OBJ*)var))) goto done;
}
if(NC_NOERR == (stat = NCJdictget(jvar,"chunks",&jvalue)) && \
jvalue && NCJsort(jvalue) == NCJ_ARRAY && NCJarraylength(jvalue) == 0 && \
xarray && nclistlength(zvar->xarray) == 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this xarray be zvar->xarray as in the following function call? I don't see xarray defined here, though the compiler's not complaining, so it must be declared somewhere.

If so, you might be able to make this if an else-if, using the previous `if (zvar->xarray == NULL) condition.

Otherwise, the code looks good, though I'm not familiar with Zarr or netCDF internals. Could you add a test for the desired behavior to ensure this change fixes the problem and that the problem stays fixed?

I'd also appreciate a more descriptive PR title, since I won't remember what issue #3108 was about while on the PR tab. Perhaps "Handle scalars in Zarr" or similar?

Also, what are your thoughts on the comment on line 1598 suggesting Zarr doesn't support scalars, is this still true? Do you think that check should be before this one? Would you still need to set shapes?

@mannreis mannreis Mar 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

xarray is declared here:

int xarray = 0;

and is set here:
if(zinfo->controls.flags & FLAG_XARRAYDIMS) {xarray = 1;}

One could also check fot zvar->xarray != NULL but nclistlength(NULL) returns 0, so there's no need.

Adding a test could be nice indeed! Will try to get that in (also contemplating the cases bellow).

Regarding Zarr not supporting scalars. I am not familiar with the specs, @DennisHeimbigner has a much better insight on it. To me it seems that if there is no dimensions for shapes or chunks in dataset.zarr/var/.zarray (or they are 0-length), then it could be a scalar. To get it's value, we check if there's at least one physical chunk on disk (dataset.zarr/var/0) or fallback into the default "FillValue".

Using zarr 2.18.2 here is an example of the 2 cases where "scalars" (length 1 arrays) are stored:

With file chunk With fill value -> no chunk

import zarr
z = zarr.open('z.zarr',mode='w')
z['pi'] = 3.1415
print(open('z.zarr/pi/.zarray','r').read())
print(len(open('z.zarr/pi/0','rb').read()))

import zarr
z = zarr.open('z-fill.zarr',mode='w')
z.ones('pi',shape=())
z.pi.fill_value =3.1415
print(open('z-fill.zarr/pi/.zarray','r').read())
print(len(open('z-fill.zarr/pi/0','rb').read()))

{
    "chunks": [],
    "compressor": null,
    "dtype": "<i8",
    "fill_value": 0,
    "filters": null,
    "order": "C",
    "shape": [],
    "zarr_format": 2
}
8

{
    "chunks": [],
    "compressor": null,
    "dimension_separator": ".",
    "dtype": "<f8",
    "fill_value": 3.1415,
    "filters": null,
    "order": "C",
    "shape": [],
    "zarr_format": 2
}
FileNotFoundError: [Errno 2] No such file or directory: 'z.zarr/pi/0'

For the 2nd case, I think even this patch is not properly filling the scalar value:

$ diff -y <(ncdump file://${PWD}/z.zarr#mode=zarr) <(ncdump file://${PWD}/z-fill.zarr#mode=zarr)
netcdf z {						      |	netcdf z-fill {
variables:							variables:
	double pi ;							double pi ;
		pi:_FillValue = 0. ;			      |			pi:_FillValue = 3.1415 ;
data:								data:

 pi = 3.1415 ;						      |	 pi = _ ;

zvar->scalar = 1;
/* Save the rank of the variable */
if ((stat = nc4_var_set_ndims(var,1)))
goto done;
}else{
/* suppress variable */
ZLOG(NCLOGWARN,"Empty shape for variable %s suppressed",var->hdr.name);
suppress = 1;
goto suppressvar;
}
}

if(zvar->scalar) {
Expand Down
4 changes: 1 addition & 3 deletions nczarr_test/ref_nulls_zarr.baseline
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
netcdf ref_nulls {
dimensions:
_scalar_ = 1 ;
variables:
int test(_scalar_) ;
int test ;
test:nul_sng = 0 ;
test:empty_sng = "" ;
test:space_sng = " " ;
Expand Down
3 changes: 1 addition & 2 deletions nczarr_test/ref_string_zarr.baseline
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
netcdf ref_string {
dimensions:
d2 = 2 ;
_scalar_ = 1 ;
variables:
string c(d2) ;
string truncated(_scalar_) ;
string truncated ;
truncated:_nczarr_maxstrlen = 4 ;
string v(d2) ;

Expand Down
56 changes: 55 additions & 1 deletion nczarr_test/run_corrupt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,61 @@ testnoshape2() {
fileurl="https://storage.googleapis.com/cmip6/CMIP6/CMIP/NASA-GISS/GISS-E2-1-G/historical/r1i1p1f1/day/tasmin/gn/v20181015/#mode=zarr,s3&aws.profile=no"
${NCDUMP} -h $flags $fileurl > tmp_noshape2_gs.cdl
}



test_numpy_scalars() {
# in zarr + numpy an adimensional array is a scalar - check issue #3108
# it tests these cases
# >>> np.array(1).shape
# ()
# >>> np.array(1.).shape
# ()

name='numpy_scalars.zarr'

mkdir -p $name
cat > "$name/.zmetadata" <<-'EOF'
{
"metadata": {
".zgroup": { "zarr_format": 2 },
"float_scalar/.zarray": {
"chunks": [], "compressor": null, "dtype": "<f8", "fill_value": 0.0, "filters": null, "order": "C",
"shape": [], "zarr_format": 2
},
"int_scalar/.zarray": {
"chunks": [], "compressor": null, "dtype": "<i8", "fill_value": 0, "filters": null, "order": "C",
"shape": [], "zarr_format": 2
}
},
"zarr_consolidated_format": 1
}
EOF

cat > expected_${name}.out <<-'EOF'
netcdf numpy_scalars {
variables:
double float_scalar ;
float_scalar:_FillValue = 0. ;
int64 int_scalar ;
int_scalar:_FillValue = 0LL ;
}
EOF

# Ensure variable is shown
${NCDUMP} -h ${flags} "file://${name}#mode=zarr,file,consolidated" > result_${name}.out

diff -w result_${name}.out expected_${name}.out

# Test read and write
if test "x${FEATURE_HDF5}" = xyes; then
${NCCOPY} "file://${name}#mode=zarr,file,consolidated" "${name}.nc"
${NCDUMP} -n numpy_scalars -h "${name}.nc" > copy_result_${name}.out
diff -w copy_result_${name}.out expected_${name}.out
fi
}

# run tests
test_numpy_scalars
testnoshape1
if test "x$FEATURE_S3TESTS" = xyes && test "x$FEATURE_S3_INTERNAL" = xyes ; then
# The aws-sdk-cpp driver does not support google storage
Expand Down
13 changes: 4 additions & 9 deletions nczarr_test/run_scalar.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@ s3isolate "testdir_scalar"
THISDIR=`pwd`
cd $ISOPATH

# This shell script tests support for the NC_STRING type

zarrscalar() {
rm -f $2
sed -e '/dimensions:/d' -e '/_scalar_ =/d' -e '/int v/ s|(_scalar_)||' <$1 >$2
}
# This shell script tests support the scalars

testcase() {
zext=$1
Expand Down Expand Up @@ -49,12 +44,12 @@ echo "*** read nczarr"
${NCDUMP} -n ref_scalar $nczarrurl > tmp_scalar_nczarr_${zext}.cdl
${ZMD} -h $nczarrurl > tmp_scalar_nczarr_${zext}.txt

echo "*** verify"
echo "*** verify nczarr"
diff -bw $top_srcdir/nczarr_test/ref_scalar_nczarr.cdl tmp_scalar_nczarr_${zext}.cdl

echo "*** verify zarr"
# Fixup
zarrscalar tmp_scalar_zarr_${zext}.cdl tmp_rescale_zarr_${zext}.cdl
diff -bw $top_srcdir/nczarr_test/ref_scalar.cdl tmp_rescale_zarr_${zext}.cdl
diff -bw $top_srcdir/nczarr_test/ref_scalar.cdl tmp_scalar_zarr_${zext}.cdl
}

testcase file
Expand Down
Loading