From d8cf2882c84b31609dd0f3e70fc815c9b06628a3 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Thu, 20 Aug 2020 17:30:03 -0700 Subject: [PATCH 1/4] Fixes error message for read-only attributes #408 --- Lib/__init__.py | 1 + Lib/dataset.py | 8 +++++++- Lib/fvariable.py | 7 +------ Src/Cdunifmodule.c | 14 +++++++++----- tests/test_dataset_io.py | 31 +++++++++++++++++++++++++++++++ 5 files changed, 49 insertions(+), 12 deletions(-) diff --git a/Lib/__init__.py b/Lib/__init__.py index f122245b..ffd877aa 100644 --- a/Lib/__init__.py +++ b/Lib/__init__.py @@ -4,6 +4,7 @@ # Errors from .error import CDMSError # noqa +from .Cdunif import ReadOnlyKeyError from lazy_object_proxy import Proxy from . import dataset from . import selectors diff --git a/Lib/dataset.py b/Lib/dataset.py index 41230d03..c402b44c 100644 --- a/Lib/dataset.py +++ b/Lib/dataset.py @@ -2105,7 +2105,13 @@ def createVariableCopy(self, var, id=None, attributes=None, axes=None, extbounds if attname not in ["id", "datatype", "parent"]: if isinstance(attval, string_types): attval = str(attval) - setattr(newvar, str(attname), attval) + try: + setattr(newvar, str(attname), attval) + except Cdunif.ReadOnlyKeyError as e: + # Suppress the exception context + err = "Cannot write read-only attribute '{!s}', must be " \ + "removed from '{!s}' variable before writing to file".format(e, var.id) + raise CDMSError(err) from None if (attname == "_FillValue") or (attname == "missing_value"): setattr(newvar, "_FillValue", attval) setattr(newvar, "missing_value", attval) diff --git a/Lib/fvariable.py b/Lib/fvariable.py index a3625c2d..ddf5c70b 100644 --- a/Lib/fvariable.py +++ b/Lib/fvariable.py @@ -157,12 +157,7 @@ def __setattr__(self, name, value): if hasattr(self, "parent") and self.parent is None: raise CDMSError(FileClosedWrite + self.id) if (name not in self.__cdms_internals__) and (value is not None): - try: - setattr(self._obj_, str(name), value) - except Exception: - raise CDMSError( - "Setting %s.%s=%s" % - (self.id, name, repr(value))) + setattr(self._obj_, str(name), value) self.attributes[name] = value self.__dict__[name] = value diff --git a/Src/Cdunifmodule.c b/Src/Cdunifmodule.c index 751feefc..0172042f 100644 --- a/Src/Cdunifmodule.c +++ b/Src/Cdunifmodule.c @@ -71,6 +71,7 @@ PyThread_type_lock Cdunif_lock; #endif static PyObject *CdunifError; +static PyObject *ReadOnlyKeyError; /* Set error string */ static void Cdunif_seterror(void) { @@ -2067,10 +2068,10 @@ static int PyCdunifFile_SetAttribute(PyCdunifFileObject *self, PyObject *nameobj char *name = PyStr_AsString(nameobj); if (check_if_open(self, 1)) { if (strcmp(name, "dimensions") == 0 - || strcmp(name, "variables") == 0 + || strcmp(name, "variables") == 0 || strcmp(name, "dimensioninfo") == 0 || strcmp(name, "__dict__") == 0) { - PyErr_SetString(PyExc_TypeError, "object has read-only attributes"); + PyErr_Format(ReadOnlyKeyError, "%s", name); return -1; } define_mode(self, 1); @@ -2423,9 +2424,10 @@ static int PyCdunifVariable_SetAttribute(PyCdunifVariableObject *self, PyObject *nameobj, PyObject *value) { char *name = PyStr_AsString(nameobj); if (check_if_open(self->file, 1)) { - if (strcmp(name, "shape") == 0 || strcmp(name, "dimensions") == 0 + if (strcmp(name, "shape") == 0 + || strcmp(name, "dimensions") == 0 || strcmp(name, "__dict__") == 0) { - PyErr_SetString(PyExc_TypeError, "object has read-only attributes"); + PyErr_Format(ReadOnlyKeyError, "%s", name); return -1; } define_mode(self->file, 1); @@ -3456,9 +3458,11 @@ MODULE_INIT_FUNC (Cdunif) { PyCapsule_New((void *) PyCdunif_API, "_C_API", NULL)); - CdunifError = PyStr_FromString("CdunifError"); + CdunifError = PyStr_FromString("CdunifError"); + ReadOnlyKeyError = PyErr_NewException("Cdunif.ReadOnlyKeyError", NULL, NULL); PyDict_SetItemString(d, "CdunifError", CdunifError); + PyDict_SetItemString(d, "ReadOnlyKeyError", ReadOnlyKeyError); /* Check for errors */ if (PyErr_Occurred()) diff --git a/tests/test_dataset_io.py b/tests/test_dataset_io.py index d3ff7ffd..e23bbecf 100644 --- a/tests/test_dataset_io.py +++ b/tests/test_dataset_io.py @@ -3,6 +3,9 @@ import string import os import sys +import cdat_info +import subprocess +import shutil cdms2.setNetcdfUseParallelFlag(0) @@ -21,6 +24,34 @@ def setUp(self): def testFileAttributes(self): self.assertEqual(self.file.id, "test") + def testWriteReadOnlyAttribute(self): + out = self.getTempFile("read_only.nc", "w") + out.write(self.u) + out.close() + + temp = self.getTempFile("read_only.nc", "a") + + with self.assertRaises(cdms2.ReadOnlyKeyError): + setattr(temp, "dimensions", "test") + + with self.assertRaises(cdms2.ReadOnlyKeyError): + setattr(temp["u"], "dimensions", "test") + + def testWriteThroughReadOnlyAtttribute(self): + iname = os.path.join(cdat_info.get_sampledata_path(), "clt.nc") + oname = os.path.join(os.getcwd(), "clt-modded.nc") + + shutil.copyfile(iname, oname) + + subprocess.run(["ncatted", "-a", "dimensions,clt,c,c,'lat lon'", oname]) + + ifile = cdms2.open(oname) + data = ifile("clt") + ifile.close() + + with cdms2.open(oname, "w") as ofile, self.assertRaises(cdms2.CDMSError): + ofile.write(data) + def testScalarSlice(self): u = self.u scalar = u[0,0,0] From 916b1cb80bb6e839aa77c5549831e0dd626ccca2 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Fri, 28 Aug 2020 14:01:48 -0700 Subject: [PATCH 2/4] Fixes missing dependency --- Lib/__init__.py | 5 ++--- Lib/error.py | 2 ++ Makefile | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/__init__.py b/Lib/__init__.py index 1fb6c7c0..bb44efbc 100644 --- a/Lib/__init__.py +++ b/Lib/__init__.py @@ -4,7 +4,8 @@ # Errors from .error import CDMSError # noqa -from .Cdunif import ReadOnlyKeyError +from .error import CdunifError # noqa +from .error import ReadOnlyKeyError # noqa from lazy_object_proxy import Proxy from . import dataset from . import selectors @@ -138,5 +139,3 @@ from . import dask_protocol # noqa except BaseException: pass - -from .Cdunif import CdunifError diff --git a/Lib/error.py b/Lib/error.py index ba970ad0..983ef39e 100644 --- a/Lib/error.py +++ b/Lib/error.py @@ -1,3 +1,5 @@ "Error object for cdms module" from .Cdunif import CDMSError # noqa: F401 +from .Cdunif import CdunifError # noqa: F401 +from .Cdunif import ReadOnlyKeyError # noqa: F401 diff --git a/Makefile b/Makefile index aa9ae2f9..1d15bc91 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ label ?= nightly build_script = conda-recipes/build_tools/conda_build.py -test_pkgs = testsrunner pytest +test_pkgs = testsrunner pytest nco last_stable ?= 3.1.4 conda_build_env ?= build-$(pkg_name) From dcf21e4e3d3a3e5f85d30aeec15866856d2e3cf1 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Tue, 23 Feb 2021 18:10:49 -0800 Subject: [PATCH 3/4] Adds missing test dependency and quick make targets for debugging --- Makefile | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Makefile b/Makefile index 4211143a..fe207850 100644 --- a/Makefile +++ b/Makefile @@ -97,6 +97,14 @@ build: install-conda prep-feedstock create-conda-env EXTRA_CB_OPTIONS='--croot $(LOCAL_CHANNEL_DIR)' \ $(SCRIPTS_DIR)/build_steps.sh | tee $(WORK_DIR)/`cat $(PWD)/.variant` +.PHONY: build-env +build-env: + $(CONDA_ENV); \ + $(CONDA_ACTIVATE) base; \ + $(CONDA_RC); \ + conda activate build; \ + bash + .PHONY: build-docs build-docs: ENV := docs build-docs: CHANNELS := -c file://$(LOCAL_CHANNEL_DIR) -c conda-forge @@ -118,6 +126,7 @@ clean-docs: .PHONY: test test: ENV := test test: CHANNELS := -c file://$(LOCAL_CHANNEL_DIR) -c conda-forge -c cdat/label/nightly +test: CONDA_TEST_PACKAGES += nco test: create-conda-env [[ ! -e "$(TEST_OUTPUT_DIR)" ]] && mkdir -p $(TEST_OUTPUT_DIR) || true @@ -134,6 +143,18 @@ test: create-conda-env cp -rf $(PWD)/tests_html $(TEST_OUTPUT_DIR)/ +.PHONY: test-env +test-env: + $(CONDA_ENV); \ + $(CONDA_ACTIVATE) base; \ + $(CONDA_RC); \ + conda activate test; \ + bash + +.PHONY: test-clean +test-clean: + rm -rf $(ENVS_DIR)/test + .PHONY: upload upload: ENV := upload upload: PACKAGES := 'python=3.8' anaconda-client From acf4b52cd4258f54e3ad747f0b63f6d990535ca9 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Tue, 23 Feb 2021 19:04:11 -0800 Subject: [PATCH 4/4] Fixes concatenating an cli variable --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index fe207850..53346dbf 100644 --- a/Makefile +++ b/Makefile @@ -126,7 +126,6 @@ clean-docs: .PHONY: test test: ENV := test test: CHANNELS := -c file://$(LOCAL_CHANNEL_DIR) -c conda-forge -c cdat/label/nightly -test: CONDA_TEST_PACKAGES += nco test: create-conda-env [[ ! -e "$(TEST_OUTPUT_DIR)" ]] && mkdir -p $(TEST_OUTPUT_DIR) || true @@ -135,7 +134,7 @@ test: create-conda-env $(CONDA_RC); \ conda config --set channel_priority strict; \ conda activate $(ENV); \ - conda install --yes $(CHANNELS) cdms2 testsrunner cdat_info pytest pip \ + conda install --yes $(CHANNELS) cdms2 testsrunner cdat_info pytest pip nco \ $(CONDA_TEST_PACKAGES); \ conda info; \ conda list --explicit > $(TEST_OUTPUT_DIR)/environment.txt; \