Repository: geofileops/geofileops
Pull request #604 (2024-12-18 13:11:34)

BUG: fix copy layer to a gpkg.zip file

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/geofileops/geofileops/pull/604?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops) Report
Attention: Patch coverage is `83.33333%` with `6 lines` in your changes missing coverage. Please review.
> Project coverage is 90.25%. Comparing base [(`5f08842`)](https://app.codecov.io/gh/geofileops/geofileops/commit/5f08842e475a5aaff2f4ace75e7bc77c9c2fe023?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops) to head [(`9107c69`)](https://app.codecov.io/gh/geofileops/geofileops/commit/9107c6988c393b5034b9740d8f5038fcb7774487?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops).
> Report is 86 commits behind head on main.

| [Files with missing lines](https://app.codecov.io/gh/geofileops/geofileops/pull/604?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops) | Patch % | Lines |
|---|---|---|
| [geofileops/util/\_ogr\_util.py](https://app.codecov.io/gh/geofileops/geofileops/pull/604?src=pr&el=tree&filepath=geofileops%2Futil%2F_ogr_util.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops#diff-Z2VvZmlsZW9wcy91dGlsL19vZ3JfdXRpbC5weQ==) | 85.71% | [5 Missing :warning: ](https://app.codecov.io/gh/geofileops/geofileops/pull/604?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops) |
| [geofileops/fileops.py](https://app.codecov.io/gh/geofileops/geofileops/pull/604?src=pr&el=tree&filepath=geofileops%2Ffileops.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops#diff-Z2VvZmlsZW9wcy9maWxlb3BzLnB5) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/geofileops/geofileops/pull/604?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #604      +/-   ##
==========================================
- Coverage   93.11%   90.25%   -2.87%     
==========================================
  Files          21       21              
  Lines        4113     4360     +247     
==========================================
+ Hits         3830     3935     +105     
- Misses        283      425     +142     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/geofileops/geofileops/pull/604?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=geofileops).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94cf46f..5d40ed2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,6 +25,7 @@
 
 ### Bugs fixed
 
+- Fix `copy_layer` to a gpkg.zip file (#604)
 - Fix `missing_ok` parameter in `remove` being ~ignored (#605)
 
 ## 0.9.1 (2024-07-18)
diff --git a/geofileops/fileops.py b/geofileops/fileops.py
index 8e01493..005fd45 100644
--- a/geofileops/fileops.py
+++ b/geofileops/fileops.py
@@ -2535,7 +2535,7 @@ def copy_layer(
         raise ValueError(f"src file doesn't exist: {src}")
     # If dest file exists already and no append
     if not append and dst.exists():
-        if force is True:
+        if force:
             remove(dst)
         else:
             logger.info(f"Output file exists already, so stop: {dst}")
diff --git a/geofileops/util/_ogr_util.py b/geofileops/util/_ogr_util.py
index fea1f1d..23dea70 100644
--- a/geofileops/util/_ogr_util.py
+++ b/geofileops/util/_ogr_util.py
@@ -548,57 +548,12 @@ def vector_translate(
 
         # Sometimes an invalid output file is written, so close and try to reopen it.
         output_ds = None
-        if output_path.exists():
-            try:
-                output_ds = gdal.OpenEx(
-                    str(output_path), nOpenFlags=gdal.OF_VECTOR | gdal.OF_UPDATE
-                )
-
-                # If the (first) output row contains NULL as geom/geometry, gdal will
-                # add an attribute column with the name of the (alias of) the geometry
-                # column: so "geometry" or "geom".
-                # To fix this, delete the "geom" or "geometry" attribute column if
-                # present if the input file didn't have an attribute column with this
-                # name.
-                # Bug documented in https://github.com/geofileops/geofileops/issues/313
-                #
-                # Remark: this check must be done on the reopened output file because
-                # in some cases the "geometrycolumn" is incorrectly listed in the field
-                # list of the Dataset returned by VectorTranslate. E.g. when the input
-                # file is empty.
-                if not input_has_geometry_attribute or not input_has_geom_attribute:
-                    assert isinstance(output_ds, gdal.Dataset)
-                    if output_layer is not None:
-                        result_layer = output_ds.GetLayer(output_layer)
-                    elif output_ds.GetLayerCount() == 1:
-                        result_layer = output_ds.GetLayerByIndex(0)
-                    else:
-                        result_layer = None
-                        logger.warning(
-                            "Unable to determine output layer, so not able to remove "
-                            "possibly incorrect geom and geometry text columns, with "
-                            f"input_path: {input_path}, output_path: {output_path}"
-                        )
-
-                    # Output layer was found, so check it
-                    if result_layer is not None:
-                        layer_defn = result_layer.GetLayerDefn()
-                        for field_idx in range(layer_defn.GetFieldCount()):
-                            name = layer_defn.GetFieldDefn(field_idx).GetName().lower()
-                            if (name == "geom" and not input_has_geom_attribute) or (
-                                name == "geometry" and not input_has_geometry_attribute
-                            ):
-                                result_layer.DeleteField(field_idx)
-                                break
-
-            except Exception as ex:
-                logger.info(
-                    f"Opening output file gave error, probably the input file was "
-                    f"empty, no rows were selected or geom was NULL: {ex}"
-                )
-                gfo.remove(output_path)
-            finally:
-                output_ds = None
+        _validate_file(
+            output_path,
+            output_layer,
+            input_has_geometry_attribute,
+            input_has_geom_attribute,
+        )
 
     except Exception as ex:
         output_ds = None
@@ -633,6 +588,100 @@ def vector_translate(
     return True
 
 
+def _validate_file(
+    path: Path,
+    layer: Optional[str],
+    input_has_geometry_attribute: bool,
+    input_has_geom_attribute: bool,
+):
+    """Check the file for invalid geometry columns and removes them.
+
+    Args:
+        path (Path): the file to check.
+        layer (Optional[str]): the output layer name.
+        input_has_geometry_attribute (bool): True if the input file has a geometry
+            attribute column.
+        input_has_geom_attribute (bool): True if the input file has a geom attribute
+            column.
+    """
+    if not path.exists():
+        return
+
+    def is_file_valid(path: Path, fix: bool) -> bool:
+        """Check if the file is valid.
+
+        Args:
+            path (Path): the file to check.
+            fix (bool): True to fix the invalid columns.
+        """
+        try:
+            # Only if fix is True, open the file in update mode
+            if fix:
+                nOpenFlags = gdal.OF_VECTOR | gdal.OF_UPDATE
+            else:
+                nOpenFlags = gdal.OF_VECTOR | gdal.OF_READONLY
+
+            output_ds = gdal.OpenEx(str(path), nOpenFlags=nOpenFlags)
+
+            # If the (first) output row contains NULL as geom/geometry, gdal will
+            # add an attribute column with the name of the (alias of) the geometry
+            # column: so "geometry" or "geom".
+            # To fix this, delete the "geom" or "geometry" attribute column if
+            # present if the input file didn't have an attribute column with this
+            # name.
+            # Bug documented in https://github.com/geofileops/geofileops/issues/313
+            #
+            # Remark: this check must be done on the reopened output file because
+            # in some cases the "geometrycolumn" is incorrectly listed in the field
+            # list of the Dataset returned by VectorTranslate. E.g. when the input
+            # file is empty.
+            if not input_has_geometry_attribute or not input_has_geom_attribute:
+                assert isinstance(output_ds, gdal.Dataset)
+                if layer is not None:
+                    result_layer = output_ds.GetLayer(layer)
+                elif output_ds.GetLayerCount() == 1:
+                    result_layer = output_ds.GetLayerByIndex(0)
+                else:
+                    result_layer = None
+                    logger.warning(
+                        "Unable to determine output layer, so not able to remove "
+                        "possibly incorrect geom and geometry text columns, with "
+                        f"path: {path}, path: {path}"
+                    )
+
+                # Output layer was found, so check it
+                if result_layer is not None:
+                    layer_defn = result_layer.GetLayerDefn()
+                    for field_idx in range(layer_defn.GetFieldCount()):
+                        name = layer_defn.GetFieldDefn(field_idx).GetName().lower()
+                        if (name == "geom" and not input_has_geom_attribute) or (
+                            name == "geometry" and not input_has_geometry_attribute
+                        ):
+                            if fix:
+                                result_layer.DeleteField(field_idx)
+                            else:
+                                return False
+
+                            break
+
+        except Exception as ex:
+            logger.warning(
+                f"Opening output file gave error, probably the input file was "
+                f"empty, no rows were selected or geom was NULL: {ex}"
+            )
+            gfo.remove(path)
+        finally:
+            output_ds = None
+
+        return True
+
+    # First check if the file has invalid geometry columns without fixing so we can
+    # open the file read-only.
+    if not is_file_valid(path, fix=False):
+        logger.warning(f"Invalid geometry columns found in {path}, try to fix...")
+        is_file_valid(path, fix=True)
+
+
 def _prepare_gdal_options(options: dict, split_by_option_type: bool = False) -> dict:
     """Prepares the options so they are ready to pass on to gdal.
 


Test patch:
diff --git a/tests/test_geofile.py b/tests/test_geofile.py
index 67e31ee..9c98fc9 100644
--- a/tests/test_geofile.py
+++ b/tests/test_geofile.py
@@ -219,6 +219,7 @@ def test_cmp(tmp_path, suffix):
     [
         *product(["polygon-parcel"], SUFFIXES_FILEOPS, SUFFIXES_FILEOPS, [None, "XYZ"]),
         ["curvepolygon", ".gpkg", ".gpkg", None],
+        ["polygon-parcel", ".gpkg", ".gpkg.zip", None],
     ],
 )
 def test_copy_layer(tmp_path, testfile, dimensions, suffix_input, suffix_output):