Skip to content
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

NCZarr is outputting fill value as an array instead of a singleton. #2017

Merged
merged 2 commits into from
Jun 7, 2021
Merged
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
1 change: 1 addition & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This file contains a high-level description of this package's evolution. Release

## 4.8.1 - TBD

* [Bug Fix] Store NCZarr fillvalue as a singleton instead of a 1-element array. See [Github #2017](https://github.com/Unidata/netcdf-c/issues/2017).
* [Bug Fixes] The netcdf-c library was incorrectly determining the scope of dimension; similar to the type scope problem. See [Github #2012](https://github.com/Unidata/netcdf-c/pull/2012) for more information.
* [Bug Fix] Re-enable DAP2 authorization testing. See [Github #2011](https://github.com/Unidata/netcdf-c/issues/2011).
* [Bug Fix] Fix bug with windows version of mkstemp that causes failure to create more than 26 temp files. See [Github #1998](https://github.com/Unidata/netcdf-c/pull/1998).
Expand Down
13 changes: 10 additions & 3 deletions libnczarr/zcvt.c
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,12 @@ NCZ_stringconvert(nc_type typeid, size_t len, void* data0, NCjson** jdatap)
/* Create a string valued json object */
if((stat = NCJnewstringn(NCJ_STRING,len,src,&jdata)))
goto done;
} else { /* for all other values, create an array of values */
if((stat = NCJnew(NCJ_ARRAY,&jdata))) goto done;
} else { /* all other cases */
if(len == 0) {stat = NC_EINVAL; goto done;}
if(len > 1) {
if((stat = NCJnew(NCJ_ARRAY,&jdata))) goto done;
} else /* return a singletone */
jdata = NULL;
for(i=0;i<len;i++) {
char* special = NULL;
double d;
Expand Down Expand Up @@ -371,7 +375,10 @@ NCZ_stringconvert(nc_type typeid, size_t len, void* data0, NCjson** jdatap)
if(special) {nullfree(str); str = strdup(special);}
jvalue->value = str;
str = NULL;
nclistpush(jdata->contents,jvalue);
if(len == 1)
jdata = jvalue;
else
nclistpush(jdata->contents,jvalue);
jvalue = NULL;
src += typelen;
}
Expand Down
70 changes: 42 additions & 28 deletions libnczarr/zsync.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ static int ncz_jsonize_atts(NCindex* attlist, NCjson** jattrsp);
static int load_jatts(NCZMAP* map, NC_OBJ* container, NCjson** jattrsp, NClist** atypes);
static int zconvert(nc_type typeid, size_t typelen, void* dst, NCjson* src);
static int computeattrinfo(const char* name, NClist* atypes, NCjson* values,
nc_type* typeidp, size_t* lenp, void** datap);
nc_type* typeidp, size_t* typelenp, size_t* lenp, void** datap);
static int parse_group_content(NCjson* jcontent, NClist* dimdefs, NClist* varnames, NClist* subgrps);
static int parse_group_content_pure(NCZ_FILE_INFO_T* zinfo, NC_GRP_INFO_T* grp, NClist* varnames, NClist* subgrps);
static int define_grp(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp);
Expand All @@ -28,7 +28,7 @@ static int locategroup(NC_FILE_INFO_T* file, size_t nsegs, NClist* segments, NC_
static int createdim(NC_FILE_INFO_T* file, const char* name, size64_t dimlen, NC_DIM_INFO_T** dimp);
static int parsedimrefs(NC_FILE_INFO_T*, NClist* dimnames, size64_t* shape, NC_DIM_INFO_T** dims, int create);
static int decodeints(NCjson* jshape, size64_t* shapes);
static int computeattrdata(nc_type* typeidp, NCjson* values, size_t* lenp, void** datap);
static int computeattrdata(nc_type* typeidp, NCjson* values, size_t* typelenp, size_t* lenp, void** datap);
static int inferattrtype(NCjson* values, nc_type* typeidp);
static int mininttype(unsigned long long u64, int negative);
static int computedimrefs(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var, int purezarr, int xarray, int ndims, NClist* dimnames, size64_t* shapes, NC_DIM_INFO_T** dims);
Expand Down Expand Up @@ -244,6 +244,7 @@ ncz_sync_var(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var)
NCjson* jncvar = NULL;
NCjson* jdimrefs = NULL;
NCjson* jtmp = NULL;
NCjson* jfill = NULL;
size64_t shape[NC_MAX_VAR_DIMS];
NCZ_VAR_INFO_T* zvar = var->format_var_info;

Expand Down Expand Up @@ -325,7 +326,6 @@ ncz_sync_var(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var)
if(!var->no_fill) {
int fillsort;
int atomictype = var->type_info->hdr.id;
NCjson* jfill = NULL;
/* A scalar value providing the default value to use for uninitialized
portions of the array, or ``null`` if no fill_value is to be used. */
/* Use the defaults defined in netdf.h */
Expand All @@ -339,9 +339,17 @@ ncz_sync_var(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var)
if((stat = nc4_get_default_fill_value(atomictype,var->fill_value))) goto done;
}
/* Convert var->fill_value to a string */
if((stat = NCZ_stringconvert(atomictype,1,var->fill_value,&jfill)))
goto done;
if((stat = NCJinsert(jvar,"fill_value",jfill))) goto done;
if((stat = NCZ_stringconvert(atomictype,1,var->fill_value,&jfill))) goto done;
if(jfill->sort == NCJ_ARRAY) { /* stringconvert should prevent this from happening */
assert(NCJlength(jfill) > 0);
if((stat = NCJarrayith(jfill,0,&jtmp))) goto done; /* use the 0th element */
if((stat = NCJclone(jtmp,&jtmp))) goto done; /* clone so we can free it later */
NCJreclaim(jfill);
jfill = jtmp;
jtmp = NULL;
}
if((stat = NCJinsert(jvar,"fill_value",jfill))) goto done;
jfill = NULL;
}

/* order key */
Expand Down Expand Up @@ -463,6 +471,7 @@ ncz_sync_var(NC_FILE_INFO_T* file, NC_VAR_INFO_T* var)
NCJreclaim(jvar);
NCJreclaim(jncvar);
NCJreclaim(jtmp);
NCJreclaim(jfill);
return THROW(stat);
}

Expand Down Expand Up @@ -683,6 +692,8 @@ ncz_sync_atts(NC_FILE_INFO_T* file, NC_OBJ* container, NCindex* attlist)
/**
@internal Convert a list of attributes to corresponding json.
Note that this does not push to the file.
Also note that attributes of length 1 are stored as singletons, not arrays.
This is to be more consistent with pure zarr.
@param attlist - [in] the attributes to dictify
@param jattrsp - [out] the json'ized att list
@return NC_NOERR
Expand Down Expand Up @@ -858,11 +869,11 @@ Extract type and data for an attribute
*/
static int
computeattrinfo(const char* name, NClist* atypes, NCjson* values,
nc_type* typeidp, size_t* lenp, void** datap)
nc_type* typeidp, size_t* typelenp, size_t* lenp, void** datap)
{
int stat = NC_NOERR;
int i;
size_t len;
size_t len, typelen;
void* data;
nc_type typeid;

Expand All @@ -880,10 +891,11 @@ computeattrinfo(const char* name, NClist* atypes, NCjson* values,
}
if(typeid >= NC_STRING)
{stat = NC_EINTERNAL; goto done;}
if((stat = computeattrdata(&typeid, values, &len, &data))) goto done;
if((stat = computeattrdata(&typeid, values, &typelen, &len, &data))) goto done;

if(typeidp) *typeidp = typeid;
if(lenp) *lenp = len;
if(typelenp) *typelenp = typelen;
if(datap) {*datap = data; data = NULL;}

done:
Expand All @@ -895,7 +907,7 @@ computeattrinfo(const char* name, NClist* atypes, NCjson* values,
Extract data for an attribute
*/
static int
computeattrdata(nc_type* typeidp, NCjson* values, size_t* lenp, void** datap)
computeattrdata(nc_type* typeidp, NCjson* values, size_t* typelenp, size_t* lenp, void** datap)
{
int stat = NC_NOERR;
size_t datalen;
Expand All @@ -911,7 +923,7 @@ computeattrdata(nc_type* typeidp, NCjson* values, size_t* lenp, void** datap)

/* Collect the length of the attribute; might be a singleton */
switch (values->sort) {
case NCJ_DICT: stat = NC_EINTERNAL; goto done;
case NCJ_DICT: stat = NC_ENCZARR; goto done;
case NCJ_ARRAY:
datalen = nclistlength(values->contents);
break;
Expand All @@ -923,21 +935,22 @@ computeattrdata(nc_type* typeidp, NCjson* values, size_t* lenp, void** datap)
break;
}

/* Allocate data space */
if((stat = NC4_inq_atomic_type(typeid, NULL, &typelen)))
goto done;
if(typeid == NC_CHAR)
data = malloc(typelen*(datalen+1));
else
data = malloc(typelen*datalen);
if(data == NULL)
{stat = NC_ENOMEM; goto done;}

/* convert to target type */
if((stat = zconvert(typeid, typelen, data, values)))
goto done;

if(datalen > 0) {
/* Allocate data space */
if((stat = NC4_inq_atomic_type(typeid, NULL, &typelen)))
goto done;
if(typeid == NC_CHAR)
data = malloc(typelen*(datalen+1));
else
data = malloc(typelen*datalen);
if(data == NULL)
{stat = NC_ENOMEM; goto done;}
/* convert to target type */
if((stat = zconvert(typeid, typelen, data, values)))
goto done;
}
if(lenp) *lenp = datalen;
if(typelenp) *typelenp = typelen;
if(datap) {*datap = data; data = NULL;}
if(typeidp) *typeidp = typeid; /* return possibly inferred type */

Expand Down Expand Up @@ -982,7 +995,7 @@ inferattrtype(NCjson* value, nc_type* typeidp)
typeid = NC_CHAR;
break;
default:
return NC_EINTERNAL;
return NC_ENCZARR;
}
if(typeidp) *typeidp = typeid;
return NC_NOERR;
Expand Down Expand Up @@ -1203,7 +1216,7 @@ ncz_read_atts(NC_FILE_INFO_T* file, NC_OBJ* container)
/* Create the attribute */
/* Collect the attribute's type and value */
if((stat = computeattrinfo(key->value,atypes,value,
&typeid,&len,&data)))
&typeid,NULL,&len,&data)))
goto done;
if((stat = ncz_makeattr(container,attlist,key->value,typeid,len,data,&att)))
goto done;
Expand Down Expand Up @@ -1430,9 +1443,10 @@ define_vars(NC_FILE_INFO_T* file, NC_GRP_INFO_T* grp, NClist* varnames)
if(jvalue == NULL)
var->no_fill = 1;
else {
size_t fvlen;
typeid = var->type_info->hdr.id;
var->no_fill = 0;
if((stat = computeattrdata(&typeid, jvalue, NULL, &var->fill_value)))
if((stat = computeattrdata(&typeid, jvalue, NULL, &fvlen, &var->fill_value)))
goto done;
assert(typeid == var->type_info->hdr.id);
/* Note that we do not create the _FillValue
Expand Down
26 changes: 13 additions & 13 deletions ncdap_test/pathcvt.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@
/*
Synopsis

pathcvt [-u|-w|-m|-c] [-e] PATH
pathcvt [-u|-w|-m|-c] [-e] [-d <driveletter>] PATH

Options
-e add backslash escapes to '\' and ' '
-d <driveletter> use driveletter when needed; defaults to 'c'
Output type options:
-u convert to Unix form of path
-w convert to Windows form of path
Expand All @@ -45,7 +46,8 @@ Default is to convert to the format used by the platform.

struct Options {
int target;
int escape;
int escapes;
int drive;
int debug;
} cvtoptions;

Expand All @@ -60,11 +62,13 @@ main(int argc, char** argv)
char* inpath;

memset((void*)&cvtoptions,0,sizeof(cvtoptions));
cvtoptions.drive = 'c';

while ((c = getopt(argc, argv, "cD:ehmuw")) != EOF) {
while ((c = getopt(argc, argv, "cD:d:ehmuw")) != EOF) {
switch(c) {
case 'c': cvtoptions.target = NCPD_CYGWIN; break;
case 'e': cvtoptions.escape = 1; break;
case 'd': cvtoptions.drive = optarg[0]; break;
case 'e': cvtoptions.escapes = 1; break;
case 'h': usage(NULL); break;
case 'm': cvtoptions.target = NCPD_MSYS; break;
case 'u': cvtoptions.target = NCPD_NIX; break;
Expand All @@ -90,8 +94,8 @@ main(int argc, char** argv)
if(cvtoptions.target == NCPD_UNKNOWN)
cvtpath = NCpathcvt(inpath);
else
cvtpath = NCpathcvt_test(inpath,cvtoptions.target,'c');
if(cvtpath && cvtoptions.escape) {
cvtpath = NCpathcvt_test(inpath,cvtoptions.target,(char)cvtoptions.drive);
if(cvtpath && cvtoptions.escapes) {
char* path = cvtpath; cvtpath = NULL;
cvtpath = escape(path);
free(path);
Expand All @@ -116,20 +120,16 @@ escape(const char* path)
const char* p;
char* q;
char* epath = NULL;
const char* escapes = " \\";

epath = (char*)malloc((2*slen) + 1);
if(epath == NULL) usage("out of memtory");
p = path;
q = epath;
for(;*p;p++) {
switch (*p) {
case '\\': case ' ':
if(strchr(escapes,*p) != NULL)
*q++ = '\\';
/* fall thru */
default:
*q++ = *p;
break;
}
*q++ = *p;
}
*q = '\0';
return epath;
Expand Down
81 changes: 36 additions & 45 deletions ncdap_test/ref_pathcvt.txt
Original file line number Diff line number Diff line change
@@ -1,45 +1,36 @@
path: /xxx/a/b
/xxx/a/b
/cygdrive/c/xxx/a/b
/c/xxx/a/b
c:\\xxx\\a\\b
path: d:/x/y
/d/x/y
/cygdrive/d/x/y
/d/x/y
d:\\x\\y
path: /cygdrive/d/x/y
/d/x/y
/cygdrive/d/x/y
/d/x/y
d:\\x\\y
path: /d/x/y
/d/x/y
/cygdrive/d/x/y
/d/x/y
d:\\x\\y
path: /cygdrive/d
/d
/cygdrive/d
/d
d:
path: /d
/d
/cygdrive/d
/d
d:
path: /cygdrive/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn
/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn
/cygdrive/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn
/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn
d:\\git\\netcdf-c\\dap4_test\\test_anon_dim.2.syn
path: d:\x\y
/d/x/y
/cygdrive/d/x/y
/d/x/y
d:\\x\\y
path: d:\x\y w\z
/d/x/y w/z
/cygdrive/d/x/y w/z
/d/x/y w/z
d:\\x\\y\\ w\\z
path: -u: |/xxx/x/y| => |/xxx/x/y|
path: -c: |/xxx/x/y| => |/cygdrive/c/xxx/x/y|
path: -m: |/xxx/x/y| => |/c/xxx/x/y|
path: -w: |/xxx/x/y| => |c:\\xxx\\x\\y|
path: -u: |d:/x/y| => |/d/x/y|
path: -c: |d:/x/y| => |/cygdrive/d/x/y|
path: -m: |d:/x/y| => |/d/x/y|
path: -w: |d:/x/y| => |d:\\x\\y|
path: -u: |/cygdrive/d/x/y| => |/d/x/y|
path: -c: |/cygdrive/d/x/y| => |/cygdrive/d/x/y|
path: -m: |/cygdrive/d/x/y| => |/d/x/y|
path: -w: |/cygdrive/d/x/y| => |d:\\x\\y|
path: -u: |/d/x/y| => |/d/x/y|
path: -c: |/d/x/y| => |/cygdrive/d/x/y|
path: -m: |/d/x/y| => |/d/x/y|
path: -w: |/d/x/y| => |d:\\x\\y|
path: -u: |/cygdrive/d| => |/d|
path: -c: |/cygdrive/d| => |/cygdrive/d|
path: -m: |/cygdrive/d| => |/d|
path: -w: |/cygdrive/d| => |d:|
path: -u: |/d| => |/d|
path: -c: |/d| => |/cygdrive/d|
path: -m: |/d| => |/d|
path: -w: |/d| => |d:|
path: -u: |/cygdrive/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn| => |/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn|
path: -c: |/cygdrive/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn| => |/cygdrive/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn|
path: -m: |/cygdrive/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn| => |/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn|
path: -w: |/cygdrive/d/git/netcdf-c/dap4_test/test_anon_dim.2.syn| => |d:\\git\\netcdf-c\\dap4_test\\test_anon_dim.2.syn|
path: -u: |d:\x\y| => |/d/x/y|
path: -c: |d:\x\y| => |/cygdrive/d/x/y|
path: -m: |d:\x\y| => |/d/x/y|
path: -w: |d:\x\y| => |d:\\x\\y|
path: -u: |d:\x\y w\z| => |/d/x/y\ w/z|
path: -c: |d:\x\y w\z| => |/cygdrive/d/x/y\ w/z|
path: -m: |d:\x\y w\z| => |/d/x/y\ w/z|
path: -w: |d:\x\y w\z| => |d:\\x\\y\ w\\z|
2 changes: 1 addition & 1 deletion ncdap_test/testauth.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ LOCALRCFILES="$WD/.dodsrc $WD/.daprc $WD/.ncrc $WD/$NETRC $WD/$NETRCIMP"
HOMERCFILES="$HOME/.dodsrc $HOME/.daprc $HOME/.ncrc $HOME/$NETRC $HOME/$NETRCIMP"
NETRCFILE=$WD/$NETRC
DAPRCFILE=$WD/$RC
if test "x$FP_ISMSVC" = x1 ; then
if test "x$FP_ISMSVC" != x ; then
LOCALRCFILES=`${execdir}/pathcvt "$LOCALRCFILES"`
HOMERCFILES=`${execdir}/pathcvt "$HOMERCFILES"`
NETRCFILE=`${execdir}/pathcvt "$NETRCFILE"`
Expand Down
Loading