fix: #868, #874, #879 (#881)

* fix(#868): bug fixed that occurred when loading packages with external file references and then changing that reference. behavior of setting all files to external changed, existing external files are no longer automatically overridden with the default file name.

* fix(external files): test updated, added reloading and saving of simulation after all data is set to external

* fix(#874): fixed data shape for one line obs files

* fix(#879): external files that start with a comment on the first line now load correctly

* fix: Added test for external file starting with comment

* fix: debug code removed

* fix: code cleanup

autotest/t504_test.py

@@ -884,6 +884,11 @@ def test027_timeseriestest():
     sim.set_all_data_external()
     sim.write_simulation()
 
+    # reload sim
+    sim = MFSimulation.load(model_name, 'mf6', exe_name, run_folder,
+                            verify_data=True)
+    sim.write_simulation()
+
     if run:
         # run simulation
         sim.run_simulation()

examples/data/mf6/test001e_UZF_3lay/chd_spd.txt

@@ -0,0 +1,3 @@
+# comment
+  3  1  1   -25.0000000     
+  3  1  10  -25.0000000   

examples/data/mf6/test001e_UZF_3lay/test001e_UZF_3lay.chd

@@ -9,6 +9,5 @@ BEGIN Dimensions
 END Dimensions
 
 BEGIN Period 1
-  3  1  1   -25.0000000     
-  3  1  10  -25.0000000     
+  OPEN/CLOSE 'chd_spd.txt'
 END Period

flopy/mf6/data/mfdata.py

@@ -386,7 +386,7 @@ def get_description(self, description=None, data_set=None):
         return description
 
     def load(self, first_line, file_handle, block_header,
-             pre_data_comments=None):
+             pre_data_comments=None, external_file_info=None):
         self.enabled = True
 
     def is_valid(self):

flopy/mf6/data/mfdataarray.py

@@ -49,12 +49,14 @@ class MFArray(MFMultiDimVar):
         Returns whether this MFArray supports layered data
     set_layered_data : (layered_data : bool)
         Sets whether this MFArray supports layered data
-    store_as_external_file : (external_file_path : string, multiplier : float,
-        layer_num : int)
+    store_as_external_file : (external_file_path : string, layer_num : int,
+                             replace_existing_external : bool)
         Stores data from layer "layer_num" to an external file at
-        "external_file_path" with a multiplier "multiplier".  For unlayered
-        data do not pass in "layer". If layer is not specified all layers
-        will be stored with each layer as a separate file.
+        "external_file_path".  For unlayered data do not pass in "layer".
+        If layer is not specified all layers will be stored with each layer
+        as a separate file. If replace_existing_external is set to False,
+        this method will not do anything if the data is already in an
+        external file.
     store_as_internal_array : (multiplier : float, layer_num : int)
         Stores data from layer "layer_num" internally within the MODFLOW file
         with a multiplier "multiplier". For unlayered data do not pass in
@@ -379,7 +381,8 @@ def make_layered(self):
                                   self._simulation_data.debug)
 
     def store_as_external_file(self, external_file_path, layer=None,
-                               binary=False):
+                               binary=False,
+                               replace_existing_external=True):
         storage = self._get_storage_obj()
         if storage is None:
             self._set_storage_obj(self._new_storage(False, True))
@@ -388,9 +391,17 @@ def store_as_external_file(self, external_file_path, layer=None,
         if layer is None:
             layer_list = []
             for index in range(0, storage.layer_storage.get_total_size()):
-                layer_list.append(index)
+                if replace_existing_external or \
+                        storage.layer_storage[index].data_storage_type == \
+                        DataStorageType.internal_array:
+                    layer_list.append(index)
         else:
-            layer_list = [layer]
+            if replace_existing_external or \
+                    storage.layer_storage[layer].data_storage_type == \
+                    DataStorageType.internal_array:
+                layer_list = [layer]
+            else:
+                layer_list = []
 
         # store data from each layer in a separate file
         for current_layer in layer_list:
@@ -579,9 +590,10 @@ def _set_data(self, data, multiplier=None, layer=None):
         self._layer_shape = storage.layer_storage.list_shape
 
     def load(self, first_line, file_handle, block_header,
-             pre_data_comments=None):
+             pre_data_comments=None, external_file_info=None):
         super(MFArray, self).load(first_line, file_handle, block_header,
-                                  pre_data_comments=None)
+                                  pre_data_comments=None,
+                                  external_file_info=None)
         self._resync()
         if self.structure.layered:
             try:
@@ -609,9 +621,14 @@ def load(self, first_line, file_handle, block_header,
         file_access = MFFileAccessArray(self.structure, self._data_dimensions,
                                         self._simulation_data, self._path,
                                         self._current_key)
+        storage = self._get_storage_obj()
         self._layer_shape, return_val = file_access.load_from_package(
-            first_line, file_handle, self._layer_shape, self._get_storage_obj(),
+            first_line, file_handle, self._layer_shape, storage,
             self._keyword, pre_data_comments=None)
+        if external_file_info is not None:
+            storage.point_to_existing_external_file(
+                external_file_info, storage.layer_storage.get_total_size() - 1)
+
         return return_val
 
     def _is_layered_aux(self):
@@ -1038,7 +1055,8 @@ def add_transient_key(self, transient_key):
                                                        transient_key)
 
     def store_as_external_file(self, external_file_path, layer=None,
-                               binary=False):
+                               binary=False,
+                               replace_existing_external=True):
         sim_time = self._data_dimensions.package_dim.model_dim[
             0].simulation_time
         num_sp = sim_time.get_num_stress_periods()
@@ -1054,7 +1072,8 @@ def store_as_external_file(self, external_file_path, layer=None,
                     fname, ext = os.path.splitext(external_file_path)
                     full_name = '{}_{}{}'.format(fname, sp+1, ext)
                     super(MFTransientArray, self).\
-                        store_as_external_file(full_name, layer, binary)
+                        store_as_external_file(full_name, layer, binary,
+                                               replace_existing_external)
 
     def get_data(self, layer=None, apply_mult=True, **kwargs):
         if self._data_storage is not None and len(self._data_storage) > 0:
@@ -1150,10 +1169,11 @@ def get_file_entry(self, key=0,
                                                             ext_file_action)
 
     def load(self, first_line, file_handle, block_header,
-             pre_data_comments=None):
+             pre_data_comments=None, external_file_info=None):
         self._load_prep(block_header)
         return super(MFTransientArray, self).load(first_line, file_handle,
-                                                  pre_data_comments)
+                                                  pre_data_comments,
+                                                  external_file_info)
 
     def _new_storage(self, set_layers=True, base_storage=False,
                      stress_period=0):

flopy/mf6/data/mfdatalist.py

@@ -94,7 +94,9 @@ class MFList(mfdata.MFMultiDimVar, DataListInterface):
         data do not pass in "layer".
     store_as_external_file : (external_file_path : str, binary : bool)
         store all data externally in file external_file_path. the binary
-        allows storage in a binary file.
+        allows storage in a binary file. If replace_existing_external is set
+        to False, this method will not do anything if the data is already in
+        an external file.
 
     See Also
     --------
@@ -229,15 +231,22 @@ def new_simulation(self, sim_data):
 
         self._data_line = None
 
-    def store_as_external_file(self, external_file_path, binary=False):
+    def store_as_external_file(self, external_file_path, binary=False,
+                               replace_existing_external=True):
         # only store data externally (do not subpackage info)
         if self.structure.construct_package is None:
-            data = self._get_data()
-            # if not empty dataset
-            if data is not None:
-                external_data = {'filename': external_file_path,
-                                 'data': self._get_data(), 'binary': binary}
-                self._set_data(external_data)
+            storage = self._get_storage_obj()
+            # check if data is already stored external
+            if replace_existing_external or storage is None or \
+                    storage.layer_storage.first_item().data_storage_type == \
+                    DataStorageType.internal_array:
+                data = self._get_data()
+                # if not empty dataset
+                if data is not None:
+                    external_data = {'filename': external_file_path,
+                                     'data': self._get_data(),
+                                     'binary': binary}
+                    self._set_data(external_data)
 
     def has_data(self):
         try:
@@ -509,7 +518,8 @@ def _get_file_entry_record(self, data_complete, mflist_line, text_line,
             for data_item in data_set.data_item_structures:
                 if data_item.is_aux:
                     try:
-                        aux_var_names = data_dim.package_dim.get_aux_variables()
+                        aux_var_names = \
+                            data_dim.package_dim.get_aux_variables()
                         if aux_var_names is not None:
                             for aux_var_name in aux_var_names[0]:
                                 if aux_var_name.lower() != 'auxiliary':
@@ -570,14 +580,15 @@ def _get_file_entry_record(self, data_complete, mflist_line, text_line,
                                 data_item, self.structure, [data_line],
                                 repeating_key=self._current_key)
                         data_val = data_line[index]
-                        if data_item.is_cellid or (data_item.possible_cellid and
-                                storage._validate_cellid([data_val], 0)):
+                        if data_item.is_cellid or (data_item.possible_cellid
+                                and storage._validate_cellid([data_val], 0)):
                             if data_item.shape is not None and \
                                     len(data_item.shape) > 0 and \
                                     data_item.shape[0] == 'ncelldim':
                                 model_grid = data_dim.get_model_grid()
                                 cellid_size = \
-                                        model_grid.get_num_spatial_coordinates()
+                                        model_grid.\
+                                            get_num_spatial_coordinates()
                                 data_item.remove_cellid(resolved_shape,
                                                         cellid_size)
                         data_size = 1
@@ -751,15 +762,19 @@ def _get_file_entry_record(self, data_complete, mflist_line, text_line,
                                                   self._simulation_data.debug)
 
     def load(self, first_line, file_handle, block_header,
-             pre_data_comments=None):
+             pre_data_comments=None, external_file_info=None):
         super(MFList, self).load(first_line, file_handle, block_header,
                                  pre_data_comments=None)
         self._resync()
         file_access = MFFileAccessList( self.structure, self._data_dimensions,
                                         self._simulation_data, self._path,
                                         self._current_key)
-        return file_access.load_from_package(
-            first_line, file_handle, self._get_storage_obj(), pre_data_comments)
+        storage = self._get_storage_obj()
+        result = file_access.load_from_package(
+            first_line, file_handle, storage, pre_data_comments)
+        if external_file_info is not None:
+            storage.point_to_existing_external_file(external_file_info, 0)
+        return result
 
     def _new_storage(self, stress_period=0):
         return DataStorage(self._simulation_data, self._model_or_sim,
@@ -1010,7 +1025,8 @@ def add_transient_key(self, transient_key):
     def data(self):
         return self.get_data()
 
-    def store_as_external_file(self, external_file_path, binary=False):
+    def store_as_external_file(self, external_file_path, binary=False,
+                               replace_existing_external=True):
         sim_time = self._data_dimensions.package_dim.model_dim[
             0].simulation_time
         num_sp = sim_time.get_num_stress_periods()
@@ -1025,7 +1041,8 @@ def store_as_external_file(self, external_file_path, binary=False):
                     fname, ext = os.path.splitext(external_file_path)
                     full_name = '{}_{}{}'.format(fname, sp+1, ext)
                     super(MFTransientList, self).\
-                        store_as_external_file(full_name, binary)
+                        store_as_external_file(full_name, binary,
+                                               replace_existing_external)
 
     def get_data(self, key=None, apply_mult=False, **kwargs):
         if self._data_storage is not None and len(self._data_storage) > 0:
@@ -1096,10 +1113,12 @@ def get_file_entry(self, key=0,
                                                            ext_file_action)
 
     def load(self, first_line, file_handle, block_header,
-             pre_data_comments=None):
+             pre_data_comments=None, external_file_info=None):
         self._load_prep(block_header)
         return super(MFTransientList, self).load(first_line, file_handle,
-                                                 pre_data_comments)
+                                                 block_header,
+                                                 pre_data_comments,
+                                                 external_file_info)
 
     def append_list_as_record(self, record, key=0):
         self._append_list_as_record_prep(record, key)

flopy/mf6/data/mfdatascalar.py

@@ -415,9 +415,10 @@ def get_file_entry(self, values_only=False, one_based=False,
                                            values)
 
     def load(self, first_line, file_handle, block_header,
-             pre_data_comments=None):
+             pre_data_comments=None, external_file_info=None):
         super(MFScalar, self).load(first_line, file_handle, block_header,
-                                   pre_data_comments=None)
+                                   pre_data_comments=None,
+                                   external_file_info=None)
         self._resync()
         file_access = MFFileAccessScalar(
             self.structure, self._data_dimensions, self._simulation_data,
@@ -606,10 +607,11 @@ def get_file_entry(self, key=None, ext_file_action=
                          self).get_file_entry(ext_file_action=ext_file_action)
 
     def load(self, first_line, file_handle, block_header,
-             pre_data_comments=None):
+             pre_data_comments=None, external_file_info=None):
         self._load_prep(block_header)
         return super(MFScalarTransient, self).load(first_line, file_handle,
-                                                   pre_data_comments)
+                                                   pre_data_comments,
+                                                   external_file_info)
 
     def _new_storage(self, stress_period=0):
         return OrderedDict()

flopy/mf6/data/mfdatastorage.py

@@ -780,8 +780,8 @@ def _set_array_layer(self, data, layer, multiplier, key):
                     data['data'] = [data['data']]
 
             if 'filename' in data:
-                multiplier, iprn, binary = self.process_open_close_line(data,
-                                                                        layer)
+                multiplier, iprn, binary = \
+                    self.process_open_close_line(data, layer)[0:3]
                 # store location to file
                 self.store_external(data['filename'], layer, [multiplier],
                                     print_format=iprn, binary=binary,
@@ -815,9 +815,7 @@ def _set_array_layer(self, data, layer, multiplier, key):
                 new_data.insert(0, 'open/close')
             else:
                 new_data = data[:]
-            multiplier, iprn, binary = self.process_open_close_line(new_data,
-                                                                    layer,
-                                                                    True)
+            self.process_open_close_line(new_data, layer, True)
             return True
         # try to resolve as internal array
         layer_storage = self.layer_storage[self._resolve_layer(layer)]
@@ -1096,14 +1094,24 @@ def store_external(self, file_path, layer=None, multiplier=None,
             else:
                 self.layer_storage[layer_new].factor = multiplier
                 self.layer_storage[layer_new].internal_data = None
+        self.set_ext_file_attributes(layer_new, file_path, print_format,
+                                     binary)
 
+    def set_ext_file_attributes(self, layer, file_path,
+                                print_format, binary):
         # point to the external file and set flags
-        self.layer_storage[layer_new].fname = file_path
-        self.layer_storage[layer_new].iprn = print_format
-        self.layer_storage[layer_new].binary = binary
-        self.layer_storage[layer_new].data_storage_type = \
+        self.layer_storage[layer].fname = file_path
+        self.layer_storage[layer].iprn = print_format
+        self.layer_storage[layer].binary = binary
+        self.layer_storage[layer].data_storage_type = \
                 DataStorageType.external_file
 
+    def point_to_existing_external_file(self, arr_line, layer):
+        multiplier, print_format, binary, \
+        data_file = self.process_open_close_line(arr_line, layer, store=False)
+        self.set_ext_file_attributes(layer, data_file, print_format, binary)
+        self.layer_storage[layer].factor = multiplier
+
     def external_to_external(self, new_external_file, multiplier=None,
                              layer=None, binary=None):
         # currently only support files containing ndarrays
@@ -1476,7 +1484,7 @@ def process_open_close_line(self, arr_line, layer, store=True):
         model_name = data_dim.package_dim.model_dim[0].model_name
         self._simulation_data.mfpath.add_ext_file(data_file, model_name)
 
-        return multiplier, print_format, binary
+        return multiplier, print_format, binary, data_file
 
     @staticmethod
     def _tupleize_data(data):