Skip to content

Commit

Permalink
Add support for Zarr string type to NCZarr
Browse files Browse the repository at this point in the history
* re: Unidata#2278
* re: Unidata#2485
* re: Unidata#2474

This PR subsumes PR Unidata#2278.
Actually is a bit an omnibus covering several issues.

## PR Unidata#2278
Add support for the Zarr string type.
Zarr strings are restricted currently to be of fixed size.
The primary issue to be addressed is to provide a way for user to
specify the size of the fixed length strings. This is handled by providing
the following new attributes special:
1. **_nczarr_default_maxstrlen** —
This is an attribute of the root group. It specifies the default
maximum string length for string types. If not specified, then
it has the value of 64 characters.
2. **_nczarr_maxstrlen** —
This is a per-variable attribute. It specifies the maximum
string length for the string type associated with the variable.
If not specified, then it is assigned the value of
**_nczarr_default_maxstrlen**.

This PR also requires some hacking to handle the existing netcdf-c NC_CHAR
type, which does not exist in zarr. The goal was to choose numpy types for
both the netcdf-c NC_STRING type and the netcdf-c NC_CHAR type such that
if a pure zarr implementation read them, it would still work and an
NC_CHAR type would be handled by zarr as a string of length 1.

For writing variables and NCZarr attributes, the type mapping is as follows:
* "|S1" for NC_CHAR.
* ">S1" for NC_STRING && MAXSTRLEN==1
* ">Sn" for NC_STRING && MAXSTRLEN==n

Note that it is a bit of a hack to use endianness, but it should be ok since for
string/char, the endianness has no meaning.

For reading attributes with pure zarr (i.e. with no nczarr
atribute types defined), they will always be interpreted as of
type NC_CHAR.

## Issue: Unidata#2474
This PR partly fixes this issue because it provided more
comprehensive support for Zarr attributes that are JSON valued expressions.
This PR still does not address the problem in that issue where the
_ARRAY_DIMENSION attribute is incorrectly set. Than can only be
fixed by the creator of the datasets.

## Issue: Unidata#2485
This PR also fixes the scalar failure shown in this issue.
It generally cleans up scalar handling.
It also adds a note to the documentation describing that
NCZarr supports scalars while Zarr does not and also how
scalar interoperability is achieved.

## Misc. Other Changes
1. Convert the nczarr special attributes and keys to be all lower case. So "_NCZARR_ATTR" now used "_nczarr_attr. Support back compatibility for the upper case names.
2. Cleanup my too-clever-by-half handling of scalars in libnczarr.
  • Loading branch information
DennisHeimbigner committed Aug 28, 2022
1 parent 9cc8831 commit 231ae96
Show file tree
Hide file tree
Showing 57 changed files with 2,077 additions and 1,002 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/run_tests_osx.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
name: Run macOS-based netCDF Tests


on: [pull_request, workflow_dispatch]
on: [push,pull_request, workflow_dispatch]

jobs:

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/run_tests_ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

name: Run Ubuntu/Linux netCDF Tests

on: [pull_request, workflow_dispatch]
on: [push,pull_request, workflow_dispatch]

jobs:

Expand Down Expand Up @@ -42,7 +42,7 @@ jobs:
wget https://support.hdfgroup.org/ftp/HDF/releases/HDF4.2.15/src/hdf-4.2.15.tar.bz2
tar -jxf hdf-4.2.15.tar.bz2
pushd hdf-4.2.15
./configure --prefix=${HOME}/environments/${{ matrix.hdf5 }} --disable-static --enable-shared --disable-fortran --disable-netcdf --with-szlib
./configure --prefix=${HOME}/environments/${{ matrix.hdf5 }} --disable-static --enable-shared --disable-fortran --disable-netcdf --with-szlib
make -j
make install -j
popd
Expand Down Expand Up @@ -164,7 +164,7 @@ jobs:

- name: Configure
shell: bash -l {0}
run: CFLAGS=${CFLAGS} LDFLAGS=${LDFLAGS} LD_LIBRARY_PATH=${LD_LIBRARY_PATH} ./configure --enable-hdf4 --enable-hdf5 --enable-dap --disable-dap-remote-tests --enable-doxygen
run: CFLAGS=${CFLAGS} LDFLAGS=${LDFLAGS} LD_LIBRARY_PATH=${LD_LIBRARY_PATH} ./configure --enable-hdf4 --enable-hdf5 --enable-dap --disable-dap-remote-tests --enable-doxygen --enable-external-server-tests
if: ${{ success() }}

- name: Look at config.log if error
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/run_tests_win_mingw.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
name: Run MSYS2, MinGW64-based Tests


on: [pull_request, workflow_dispatch]
on: [push,pull_request, workflow_dispatch]

jobs:

Expand Down
5 changes: 4 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1092,9 +1092,12 @@ ENDIF()

# Option to Enable DAP long tests, remote tests.
OPTION(ENABLE_DAP_REMOTE_TESTS "Enable DAP remote tests." ON)
OPTION(ENABLE_EXTERNAL_SERVER_TESTS "Enable external Server remote tests." OFF)
OPTION(ENABLE_DAP_LONG_TESTS "Enable DAP long tests." OFF)
SET(REMOTETESTSERVERS "remotetest.unidata.ucar.edu" CACHE STRING "test servers to use for remote test")

SET(REMOTETESTSERVERS "remotetest.unidata.ucar.edu" CACHE STRING "test servers to use for remote test")

# See if we have zlib
FIND_PACKAGE(ZLIB)

Expand Down Expand Up @@ -1731,7 +1734,7 @@ ENDIF()

# Set some of the options as advanced.
MARK_AS_ADVANCED(ENABLE_INTERNAL_DOCS VALGRIND_TESTS ENABLE_COVERAGE_TESTS )
MARK_AS_ADVANCED(ENABLE_DAP_REMOTE_TESTS ENABLE_DAP_LONG_TESTS USE_REMOTE_CDASH)
MARK_AS_ADVANCED(ENABLE_DAP_REMOTE_TESTS ENABLE_DAP_LONG_TESTS USE_REMOTE_CDASH ENABLE_EXTERNAL_SERVER_TESTS)
MARK_AS_ADVANCED(ENABLE_DOXYGEN_BUILD_RELEASE_DOCS DOXYGEN_ENABLE_TASKS ENABLE_DOXYGEN_SERVER_SIDE_SEARCH)
MARK_AS_ADVANCED(ENABLE_SHARED_LIBRARY_VERSION)

Expand Down
9 changes: 8 additions & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ This file contains a high-level description of this package's evolution. Release

## 4.9.1 - T.B.D.

* [Bug Fix] Fix support for reading arrays of HDF5 fixed size strings. See [Github #????](https://github.com/Unidata/netcdf-c/pull/????).
* [Enhancement] Add support for Zarr (fixed length) string type in nczarr. See [Github #????](https://github.com/Unidata/netcdf-c/pull/????).
* [Bug Fix] Split the remote tests into two parts: one for the remotetest server and one for all other external servers. Also add a configure option to enable the latter set. See [Github #2491](https://github.com/Unidata/netcdf-c/pull/2491).
* [Bug Fix] Fix support for reading arrays of HDF5 fixed size strings. See [Github #2462](https://github.com/Unidata/netcdf-c/pull/2466).
* [Bug Fix] Provide a default enum const when fill value does not match any enum constant for the value zero. See [Github #2462](https://github.com/Unidata/netcdf-c/pull/2462).
* [Bug Fix] Fix the json submodule symbol conflicts between libnetcdf and the plugin specific netcdf_json.h. See [Github #2448](https://github.com/Unidata/netcdf-c/pull/2448).
* [Bug Fix] Fix quantize with CLASSIC_MODEL files. See [Github #2405](https://github.com/Unidata/netcdf-c/pull/2445).
* [Enhancement] Add `--disable-quantize` option to `configure`.
* [Bug Fix] Fix CMakeLists.txt to handle all acceptable boolean values for -DPLUGIN_INSTALL_DIR. See [Github #2430](https://github.com/Unidata/netcdf-c/pull/2430).
>>>>>>> master
* [Enhancement] Provide a simple API to allow user access to the internal .rc file table: supports get/set/overwrite of entries of the form "key=value". See [Github #2408](https://github.com/Unidata/netcdf-c/pull/2408).
* [Bug Fix] Use env variable USERPROFILE instead of HOME for windows and mingw. See [Github #2405](https://github.com/Unidata/netcdf-c/pull/2405).
* [Bug Fix] Fix the nc_def_var_fletcher32 code in hdf5 to properly test value of the fletcher32 argument. See [Github #2403](https://github.com/Unidata/netcdf-c/pull/2403).
Expand All @@ -27,8 +30,12 @@ This file contains a high-level description of this package's evolution. Release
* [Enhancement] Allow the read/write of JSON-valued Zarr attributes to allow
for domain specific info such as used by GDAL/Zarr. See [Github #2278](https://github.com/Unidata/netcdf-c/pull/2278).
* [Enhancement] Turn on the XArray convention for NCZarr files by default. WARNING, this means that the mode should explicitly specify "nczarr" or "zarr" even if "xarray" or "noxarray" is specified. See [Github #2257](https://github.com/Unidata/netcdf-c/pull/2257).
<<<<<<< HEAD
* [Enhancement] Update the documentation to match the current filter capabilities See [Github #2249](https://github.com/Unidata/netcdf-c/pull/2249).
=======

* [Enhancement] Update the documentation to match the current filter capabilities. See [Github #2249](https://github.com/Unidata/netcdf-c/pull/2249).
>>>>>>> master
* [Enhancement] Support installation of pre-built standard filters into user-specified location. See [Github #2318](https://github.com/Unidata/netcdf-c/pull/2318).
* [Enhancement] Improve filter support. More specifically (1) add nc_inq_filter_avail to check if a filter is available, (2) add the notion of standard filters, (3) cleanup szip support to fix interaction with NCZarr. See [Github #2245](https://github.com/Unidata/netcdf-c/pull/2245).
* [Enhancement] Switch to tinyxml2 as the default xml parser implementation. See [Github #2170](https://github.com/Unidata/netcdf-c/pull/2170).
Expand Down
50 changes: 38 additions & 12 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -604,31 +604,52 @@ AM_CONDITIONAL(ENABLE_QUANTIZE, [test x$enable_quantize = xyes])

# --enable-dap => enable-dap4
enable_dap4=$enable_dap
AC_MSG_CHECKING([whether dap remote testing should be enabled])
AC_MSG_CHECKING([whether dap use of remotetest server should be enabled])
AC_ARG_ENABLE([dap-remote-tests],
[AS_HELP_STRING([--enable-dap-remote-tests],
[enable dap remote tests])])
test "x$enable_dap_remote_tests" = xno || enable_dap_remote_tests=yes
[AS_HELP_STRING([--disable-dap-remote-tests],
[disable dap remote tests])])
# Default off
test "x$enable_dap_remote_tests" = xyes || enable_dap_remote_tests=no
if test "x$enable_dap" = "xno" ; then
enable_dap_remote_tests=no
fi
AC_MSG_RESULT($enable_dap_remote_tests)

AC_MSG_CHECKING([whether dap use of remotetest server should be enabled])
AC_ARG_ENABLE([dap-remote-tests],
[AS_HELP_STRING([--disable-dap-remote-tests],
[disable dap remote tests])])
test "x$enable_dap_remote_tests" = xno || enable_dap_remote_tests=yes
AC_MSG_RESULT($enable_dap_remote_tests)

AC_MSG_CHECKING([whether use of external servers should be enabled])
AC_ARG_ENABLE([external-server-tests],
[AS_HELP_STRING([--enable-external-server-tests (default off)],
[enable external server tests])])
test "x$enable_external_server_tests" = xyes || enable_external_server_tests=no
AC_MSG_RESULT($enable_external_server_tests)

if test "x$enable_dap_remote_tests" = "xno" ; then
AC_MSG_NOTICE([--disable-dap_remote_tests => --disable-external-server-tests])
enable_external_server_tests=no
fi

# Default is not to do the remote authorization tests.
AC_MSG_CHECKING([whether dap remote authorization testing should be enabled (default off)])
AC_MSG_CHECKING([whether dap authorization testing should be enabled (default off)])
AC_ARG_ENABLE([dap-auth-tests],
[AS_HELP_STRING([--enable-dap-auth-tests],
[enable dap remote authorization tests])])
test "x$enable_dap_auth_tests" = xyes || enable_dap_auth_tests=no
AC_MSG_RESULT($enable_dap_auth_tests)

# dap must be enabled

if test "x$enable_dap" = "xno" ; then
enable_dap_auth_tests=no
fi
# if remote tests are disabled, then so is this
if test "x$enable_dap_remote_tests" = "xno" ; then
AC_MSG_NOTICE([--disable-dap => --disable-dap-remote-tests --disable-auth-tests --disable-external-server-tests])
enable_dap_remote_tests=no
enable_dap_auth_tests=no
enable_external_server_tests=no
fi
AC_MSG_RESULT($enable_dap_auth_tests)

# Did the user specify a list of test servers to try for remote tests?
AC_MSG_CHECKING([which remote test server(s) to use])
Expand All @@ -653,16 +674,20 @@ fi
if test "x$enable_dap_remote_tests" = xyes; then
AC_DEFINE([ENABLE_DAP_REMOTE_TESTS], [1], [if true, do remote tests])
fi
if test "x$enable_external_server_tests" = xyes; then
AC_DEFINE([ENABLE_EXTERNAL_SERVER_TESTS], [1], [if true, do remote external tests])
fi

AC_MSG_CHECKING([whether the time-consuming dap tests should be enabled (default off)])
AC_ARG_ENABLE([dap-long-tests],
[AS_HELP_STRING([--enable-dap-long-tests],
[enable dap long tests])])
test "x$enable_dap_long_tests" = xyes || enable_dap_long_tests=no
if test "x$enable_dap_remote_tests" = "xno" ; then
AC_MSG_RESULT([$enable_dap_long_tests])
if test "x$enable_dap_remote_tests" = "xno" || test "x$enable_external_server_tests" = "xno" ; then
AC_MSG_NOTICE([--disable-dap-remote|external-server-tests => --disable_dap_long_tests])
enable_dap_long_tests=no
fi
AC_MSG_RESULT([$enable_dap_long_tests])

# Control zarr storage
if test "x$enable_nczarr" = xyes ; then
Expand Down Expand Up @@ -1758,6 +1783,7 @@ AM_CONDITIONAL(ENABLE_DAP4, [test "x$enable_dap4" = xyes])
AM_CONDITIONAL(USE_STRICT_NULL_BYTE_HEADER_PADDING, [test x$enable_strict_null_byte_header_padding = xyes])
AM_CONDITIONAL(ENABLE_CDF5, [test "x$enable_cdf5" = xyes])
AM_CONDITIONAL(ENABLE_DAP_REMOTE_TESTS, [test "x$enable_dap_remote_tests" = xyes])
AM_CONDITIONAL(ENABLE_EXTERNAL_SERVER_TESTS, [test "x$enable_external_server_tests" = xyes])
AM_CONDITIONAL(ENABLE_DAP_AUTH_TESTS, [test "x$enable_dap_auth_tests" = xyes])
AM_CONDITIONAL(ENABLE_DAP_LONG_TESTS, [test "x$enable_dap_long_tests" = xyes])
AM_CONDITIONAL(USE_PNETCDF_DIR, [test ! "x$PNETCDFDIR" = x])
Expand Down
2 changes: 2 additions & 0 deletions dap4_test/test_thredds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ failure() {
setresultdir results_test_thredds

if test "x${RESET}" = x1 ; then rm -fr ${BASELINEH}/*.thredds ; fi
if test "x$FEATURE_THREDDSTEST" = x1 ; then
for f in $F ; do
makeurl "dap4://thredds-test.unidata.ucar.edu/thredds/dap4/casestudies" "$f"
echo "testing: $URL"
Expand All @@ -43,6 +44,7 @@ for f in $F ; do
cp ./results_test_thredds/${base}.thredds ${BASELINETH}/${base}.thredds
fi
done
fi # FEATURE_THREDDSTEST

echo "*** Pass"
exit 0
Expand Down
Loading

0 comments on commit 231ae96

Please sign in to comment.