more tests, and some cleanup of the Time object

general cleanup of Dataset.from_netCDF

Author: ChrisBarker-NOAA

gridded/depth.py

@@ -926,15 +926,17 @@ def from_netCDF(cls,
         typs = cls.sd_types + cls.ld_types
         available_to_create = [typ._can_create_from_netCDF(grid_file=dg, data_file=ds) for typ in typs]
         if not any(available_to_create):
-            warnings.warn('''Unable to automatically determine depth system so
-                            reverting to surface-only mode. Manually check the
-                            (depth_object).surface_index attribute and set it
-                            to the appropriate array index for your model data''', RuntimeWarning)
+            warnings.warn("Unable to automatically determine depth system so "
+                          "reverting to surface-only mode. Manually check the "
+                          "(depth_object).surface_index attribute and set it "
+                          "to the appropriate array index for your model data",
+                          RuntimeWarning)
             return cls.surf_types[0].from_netCDF(data_file=ds, grid_file=dg, **kwargs)
         else:
             typ = typs[np.argmax(available_to_create)]
             if sum(available_to_create) > 1:
-                warnings.warn('''Multiple depth systems detected. Using the first one found: {0}'''.format(typ.__repr__), RuntimeWarning)
+                warnings.warn("Multiple depth systems detected. Using the first one found: "
+                              f"{typ!r}", RuntimeWarning)
             return typ.from_netCDF(filename=filename,
                                    dataset=dataset,
                                    data_file=data_file,

gridded/gridded.py

@@ -24,7 +24,6 @@ class Dataset:
     An object that represent an entire complete dataset --
     a collection of Variables and the Grid that they are stored on.
     """
-
     def __init__(self,
                  ncfile=None,
                  grid=None,
@@ -68,35 +67,55 @@ def __init__(self,
             # raise ValueError("don't create from a file")
             warnings.warn("Creating a Dataset from a netcdfile directly is deprecated. "
                           "Please use Dataset.from_netCDF() instead. "
-                          "Or one of the utilities in gridded.io", DeprecationWarning)
+                          "Or use one of the utilities in gridded.io",
+                          DeprecationWarning)
 
         if ncfile is not None:
             if (grid is not None or
                   variables is not None or
                   attributes is not None):
                 raise ValueError("You can create a Dataset from a file, or from raw data"
                                  "but not both.")
-            self.nc_dataset = get_dataset(ncfile)
-            self.filename = self.nc_dataset.filepath()
-            self.grid = Grid.from_netCDF(filename=self.filename,
-                                         dataset=self.nc_dataset,
-                                         grid_topology=grid_topology)
-            self.variables = self._load_variables(self.nc_dataset)
-            self.attributes = get_dataset_attrs(self.nc_dataset)
-        else:  # no file passed in -- create from grid and variables
+            self._init_from_netCDF(ncfile)
+        else:  # Create from grid and variables -- this is what should usually happen.
             self.filename = None
             self.grid = grid
             self.variables = {} if variables is None else variables
             self.attributes = {} if attributes is None else attributes
 
+
+    def _init_from_netCDF(self,
+                          filename=None,
+                          grid_file=None,
+                          variable_files=None,
+                          grid_topology=None):
+        """
+        internal implementation -- users should call the .from_netCDF()
+        classmethod -- see its docstring for usage.
+
+        This is used to initialize a dataset from a netCDF file --
+        done this way, so it can be called from more than one place.
+        """
+        if (grid_file is not None) or (variable_files is not None):
+            raise NotImplementedError("Loading from separate netcdf files is not yet supported")
+
+        self.nc_dataset = get_dataset(filename)
+        self.filename = self.nc_dataset.filepath()
+        self.grid = Grid.from_netCDF(filename=self.filename,
+                                     dataset=self.nc_dataset,
+                                     grid_topology=grid_topology)
+        # fixme: this should load the depth and time, and then the variables.
+        self.variables = self._variables_from_netCDF(self.nc_dataset)
+        self.attributes = get_dataset_attrs(self.nc_dataset)
+
+
     @classmethod
     def from_netCDF(cls,
                     filename=None,
                     grid_file=None,
                     variable_files=None,
                     grid_topology=None):
         """
-
         NOTE: only loading from a single file is currently implemented.
               you can create a DATaset by hand, by loading the grid and
               variables separately, and then adding them
@@ -117,20 +136,13 @@ def from_netCDF(cls,
         :type grid_topology: mapping with keys of topology components and values are
                              variable names.
         """
-        if (grid_file is not None) or (variable_files is not None):
-            raise NotImplementedError("Loading from separate netcdf files is not yet supported")
-
-        # create an empty DAtaset:
+        # create an empty Dataset:
         ds = cls()
-
-        ds.nc_dataset = get_dataset(filename)
-        ds.filename = ds.nc_dataset.filepath()
-        ds.grid = Grid.from_netCDF(filename=ds.filename,
-                                     dataset=ds.nc_dataset,
-                                     grid_topology=grid_topology)
-        ds.variables = ds._load_variables(ds.nc_dataset)
-        ds.attributes = get_dataset_attrs(ds.nc_dataset)
-
+        # initialize it
+        ds._init_from_netCDF(filename,
+                             grid_file,
+                             variable_files,
+                             grid_topology)
         return ds
 
     def __getitem__(self, key):
@@ -147,10 +159,15 @@ def __str__(self):
         return descp
 
 
-    def _load_variables(self, ds):
+    def _variables_from_netCDF(self, ds):
         """
         load up the variables in the nc file
+
+        :param ds: initialized netCDF dataset
         """
+        # fixme: this needs work
+        #        It *should* have already gotten the grid and depth and time,
+        #        and then the variables can be loaded directly.
         variables = {}
         for k in ds.variables.keys():
             # find which netcdf variables are used to define the grid

gridded/tests/test_dataset.py

@@ -6,30 +6,45 @@
 import netCDF4 as nc
 
 from gridded import Dataset
-from .utilities import get_test_file_dir
+from gridded.grids import Grid_S
+from .utilities import TEST_DATA
 
-test_dir = get_test_file_dir()
 
 # Need to hook this up to existing test data infrastructure
 # ... and add more infrastructure
 
-sample_sgrid_file = os.path.join(test_dir, 'staggered_sine_channel.nc')
-arakawa_c_file = os.path.join(test_dir, 'arakawa_c_test_grid.nc')
+sample_sgrid_file = TEST_DATA / 'staggered_sine_channel.nc'
+arakawa_c_file = TEST_DATA / 'arakawa_c_test_grid.nc'
 
 
 def test_load_sgrid():
-    """ tests you can intitilize an conforming sgrid file"""
-    sinusoid = Dataset(sample_sgrid_file)
+    """ tests you can initialize an conforming sgrid file"""
+    sinusoid = Dataset.from_netCDF(sample_sgrid_file)
+
+    assert isinstance(sinusoid.grid, Grid_S)
 
     assert True  # just to make it a test
 
 
+def test_init_from_netcdf_file_directly():
+    """
+    This should raise a deprecation warning, but still work
+    """
+    with pytest.warns(DeprecationWarning):
+        gds = Dataset(arakawa_c_file)
+
+    print(gds.info)
+
+    assert isinstance(gds.grid, Grid_S)
+    assert len(gds.variables) == 6
+
+
 def test_info():
     """
     Make sure the info property is working
     This doesn't test much -- jsut tht it won't crash
     """
-    gds = Dataset(sample_sgrid_file)
+    gds = Dataset.from_netCDF(sample_sgrid_file)
 
     info = gds.info
 
@@ -40,15 +55,15 @@ def test_info():
     assert "attributes:" in info
 
 def test_get_variable_by_attribute_one_there():
-    gds = Dataset(arakawa_c_file)
+    gds = Dataset.from_netCDF(arakawa_c_file)
 
     vars = gds.get_variables_by_attribute('long_name', 'v-momentum component')
 
     assert len(vars) == 1
     assert vars[0].attributes['long_name'] == 'v-momentum component'
 
 def test_get_variable_by_attribute_multiple():
-    gds = Dataset(arakawa_c_file)
+    gds = Dataset.from_netCDF(arakawa_c_file)
 
     vars = gds.get_variables_by_attribute('units', 'meter second-1')
 
@@ -62,7 +77,7 @@ def test_get_variable_by_attribute_not_there():
     """
     This should return an empty list
     """
-    gds = Dataset(arakawa_c_file)
+    gds = Dataset.from_netCDF(arakawa_c_file)
 
     var = gds.get_variables_by_attribute('some_junk', 'more_junk')
 

gridded/tests/utilities.py

@@ -17,6 +17,7 @@
 
 HERE = Path(__file__).parent
 EXAMPLE_DATA = HERE / "example_data"
+TEST_DATA = HERE / "test_data"
 
 # # Files on PYGNOME server -- add them here as needed
 data_file_cache = pooch.create(

gridded/time.py

@@ -168,15 +168,16 @@ def __init__(self,
             raise TimeSeriesError("Time sequence has duplicate entries")
         super(Time, self).__init__(*args, **kwargs)
 
-    @classmethod
-    def locate_time_var_from_var(cls, datavar):
+
+    @staticmethod
+    def locate_time_var_from_var(datavar):
         if hasattr(datavar, 'time') and datavar.time in datavar._grp.dimensions.keys():
             varname = datavar.time
         else:
             varname = datavar.dimensions[0] if 'time' in datavar.dimensions[0] else None
-        
+
         return varname
-        
+
 
     @classmethod
     def from_netCDF(cls,
@@ -432,14 +433,15 @@ def time_in_bounds(self, time):
     def valid_time(self, time):
         """
         Raises a OutOfTimeRangeError if time is not within the bounds of the timeseries
+
+        :param time: a datetime object that you want to check.
         """
         # if time < self.min_time or time > self.max_time:
         if not self.time_in_bounds(time):
-            raise OutOfTimeRangeError(f'time specified ({time.strftime('%c')}) is not within the bounds of '
-                                      f'({self.min_time.strftime('%c')} to {self.max_time.strftime('%c')})'
+            raise OutOfTimeRangeError(f'time specified: ({time.isoformat()}) is not within the bounds of '
+                                      f'({self.min_time.isoformat()} to {self.max_time.isoformat()})'
                                       )
 
-
     def index_of(self, time, extrapolate=False):
         '''
         Returns the index of the provided time with respect to the time intervals in the file.