Repository: geofileops/geofileops (python)

Issue:
BUG: in copy_layer and append_to the default dst_layer is not the stem of the filename
The documentation states it should work like this, and this is about the only function in `geofileops` where this is the case, which doesn't give a very consistent feeling.

It is a non-backwards compatible change, but in most cases this will probably don't give any issues.

Pull request:
BUG: the default dst_layer in copy_layer and append_to is not the stem of the filename
Resolves #648

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 65f49f1..0821ce2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@
 - `erase` was renamed to `difference`, as most other open source applications/libraries
   use this terminology. `erase` just keeps existing for backwards compatibility for now,
   but a warning is shown that it might be removed in the (distant) future. (#595)
+- in `copy_layer` and `append_to` the default `dst_layer` was, contrary to the
+  documentation, not the stem of the destination filename. This is corrected now. (#648)
 
 ### Improvements
 
diff --git a/geofileops/fileops.py b/geofileops/fileops.py
index 9edeb1c..29738ee 100644
--- a/geofileops/fileops.py
+++ b/geofileops/fileops.py
@@ -2313,9 +2313,9 @@ def append_to(
     dst_dimensions: Optional[str] = None,
     options: dict = {},
 ):
-    """Append src file to the dst file.
+    """Append a layer of the source file to the destination file.
 
-    If an sql_stmt is specified, the sqlite query can contain following placeholders
+    If an `sql_stmt` is specified, the sqlite query can contain following placeholders
     that will be automatically replaced for you:
 
       * {geometrycolumn}: the column where the primary geometry is stored.
@@ -2346,8 +2346,10 @@ def append_to(
     Args:
         src (Union[str,): source file path.
         dst (Union[str,): destination file path.
-        src_layer (str, optional): source layer. Defaults to None.
-        dst_layer (str, optional): destination layer. Defaults to None.
+        src_layer (str, optional): the source layer. If None and there is only one layer
+            in the src file, that layer is taken. Defaults to None.
+        dst_layer (str, optional): the destination layer. If None, the destination file
+            stem is used as layer name. Defaults to None.
         src_crs (str, optional): an epsg int or anything supported
             by the OGRSpatialReference.SetFromUserInput() call, which includes
             an EPSG string (eg. "EPSG:4326"), a well known text (WKT) CRS
@@ -2414,6 +2416,8 @@ def append_to(
     # Check/clean input params
     src = Path(src)
     dst = Path(dst)
+    if dst_layer is None:
+        dst_layer = get_default_layer(dst)
     if force_output_geometrytype is not None:
         force_output_geometrytype = GeometryType(force_output_geometrytype)
 
@@ -2495,6 +2499,8 @@ def _append_to_nolock(
     if isinstance(columns, str):
         # If a string is passed, convert to list
         columns = [columns]
+    if dst_layer is None:
+        dst_layer = get_default_layer(dst)
 
     options = _ogr_util._prepare_gdal_options(options)
     if (
@@ -2640,9 +2646,10 @@ def copy_layer(
     append: bool = False,
     force: bool = False,
 ):
-    """Read a layer from a source file and write it to a new destination file.
+    """Copy a layer from a source file to a destination file.
 
-    Typically used to convert from one fileformat to another or to reproject.
+    Typically used to convert from one fileformat to another, to reproject or to export
+    a subset of the data using the `where` parameter.
 
     The options parameter can be used to pass any type of options to GDAL in
     the following form:
@@ -2662,7 +2669,7 @@ def copy_layer(
         dst (PathLike): The destination file path.
         src_layer (str, optional): The source layer. If None and there is only
             one layer in the src file, that layer is taken. Defaults to None.
-        dst_layer (str, optional): The destination layer. If None, the file
+        dst_layer (str, optional): The destination layer. If None, the destination file
             stem is taken as layer name. Defaults to None.
         src_crs (Union[str, int], optional): an epsg int or anything supported
             by the OGRSpatialReference.SetFromUserInput() call, which includes
@@ -2740,8 +2747,10 @@ def copy_layer(
         else:
             logger.info(f"Output file exists already, so stop: {dst}")
             return
+    if dst_layer is None:
+        dst_layer = get_default_layer(dst)
 
-    # Convert
+    # Copy
     logger.info(f"Copy layer from {src} to {dst}")
     _append_to_nolock(
         src,
diff --git a/geofileops/util/_geoops_gpd.py b/geofileops/util/_geoops_gpd.py
index 12ed06a..474fd86 100644
--- a/geofileops/util/_geoops_gpd.py
+++ b/geofileops/util/_geoops_gpd.py
@@ -884,6 +884,8 @@ def _apply_geooperation_to_layer(
                             fileops._append_to_nolock(
                                 src=tmp_partial_output_path,
                                 dst=tmp_output_path,
+                                src_layer=output_layer,
+                                dst_layer=output_layer,
                                 explodecollections=explodecollections,
                                 create_spatial_index=False,
                                 force_output_geometrytype=force_output_geometrytype,
@@ -1740,6 +1742,8 @@ def _dissolve_polygons_pass(
                         fileops._append_to_nolock(
                             src=output_notonborder_tmp_partial_path,
                             dst=output_notonborder_path,
+                            src_layer=output_layer,
+                            dst_layer=output_layer,
                             create_spatial_index=False,
                         )
                         gfo.remove(output_notonborder_tmp_partial_path)
@@ -1755,6 +1759,8 @@ def _dissolve_polygons_pass(
                         fileops._append_to_nolock(
                             src=output_onborder_tmp_partial_path,
                             dst=output_onborder_path,
+                            src_layer=output_layer,
+                            dst_layer=output_layer,
                             create_spatial_index=False,
                         )
                         gfo.remove(output_onborder_tmp_partial_path)
@@ -1764,7 +1770,7 @@ def _dissolve_polygons_pass(
                 message = f"Error executing {batches[batch_id]}: {ex}"
                 logger.exception(message)
                 calculate_pool.shutdown()
-                raise Exception(message) from ex
+                raise RuntimeError(message) from ex
 
             # Log the progress and prediction speed
             _general_util.report_progress(
@@ -2269,7 +2275,7 @@ def _add_orderby_column(path: Path, layer: str, name: str):
     # Prepare the expression to calculate the orderby column.
     # In a spatial file, a spatial order will make later use more efficiënt,
     # so use a geohash.
-    layerinfo = gfo.get_layerinfo(path)
+    layerinfo = gfo.get_layerinfo(path, layer=layer)
     if layerinfo.crs is not None and layerinfo.crs.is_geographic:
         # If the coordinates are geographic (in lat/lon degrees), ok
         expression = f"ST_GeoHash({layerinfo.geometrycolumn}, 10)"
@@ -2293,6 +2299,8 @@ def _add_orderby_column(path: Path, layer: str, name: str):
                 )*{to_geographic_factor_approx}, 4326), 10)"""
 
     # Now we can actually add the column.
-    gfo.add_column(path=path, name=name, type=gfo.DataType.TEXT, expression=expression)
+    gfo.add_column(
+        path=path, layer=layer, name=name, type=gfo.DataType.TEXT, expression=expression
+    )
     sqlite_stmt = f'CREATE INDEX {name}_idx ON "{layer}"({name})'
     gfo.execute_sql(path=path, sql_stmt=sqlite_stmt)
diff --git a/geofileops/util/_geoops_sql.py b/geofileops/util/_geoops_sql.py
index eb44f2a..cd5e527 100644
--- a/geofileops/util/_geoops_sql.py
+++ b/geofileops/util/_geoops_sql.py
@@ -3473,6 +3473,8 @@ def _two_layer_vector_operation(
                     fileops._append_to_nolock(
                         src=tmp_partial_output_path,
                         dst=tmp_output_path,
+                        src_layer=output_layer,
+                        dst_layer=output_layer,
                         explodecollections=explodecollections,
                         force_output_geometrytype=force_output_geometrytype,
                         where=where_post,


Test patch:
diff --git a/tests/test_geofile.py b/tests/test_geofile.py
index 52047a7..64cd2ab 100644
--- a/tests/test_geofile.py
+++ b/tests/test_geofile.py
@@ -15,7 +15,7 @@ from pygeoops import GeometryType
 
 import geofileops as gfo
 from geofileops import fileops
-from geofileops.util import _geofileinfo, _geoseries_util, _io_util, _ogr_util
+from geofileops.util import _geofileinfo, _geoseries_util, _io_util
 from geofileops.util._geofileinfo import GeofileInfo
 from tests import test_helper
 from tests.test_helper import SUFFIXES_FILEOPS, assert_geodataframe_equal
@@ -184,22 +184,17 @@ def test_append_different_columns(tmp_path, suffix):
         "polygon-parcel", dst_dir=tmp_path, suffix=suffix
     )
     dst_path = tmp_path / f"dst{suffix}"
-    gfo.copy(src_path, dst_path)
+    gfo.copy_layer(src_path, dst_path)
     gfo.add_column(src_path, name="extra_col", type=gfo.DataType.INTEGER)
 
-    # For CSV files, the append fails
-    if suffix == ".csv":
-        with pytest.raises(_ogr_util.GDALError):
-            gfo.append_to(src_path, dst_path)
-        return
-
-    # For other file types, all rows are appended tot the dst layer, but the extra
-    # column is not!
+    # All rows are appended tot the dst layer, but the extra column is not!
     gfo.append_to(src_path, dst_path)
 
     # Check results
-    src_info = gfo.get_layerinfo(src_path)
-    res_info = gfo.get_layerinfo(dst_path)
+    raise_on_nogeom = False if suffix == ".csv" else True
+
+    src_info = gfo.get_layerinfo(src_path, raise_on_nogeom=raise_on_nogeom)
+    res_info = gfo.get_layerinfo(dst_path, raise_on_nogeom=raise_on_nogeom)
     assert (src_info.featurecount * 2) == res_info.featurecount
     assert len(src_info.columns) == len(res_info.columns) + 1
 
@@ -278,6 +273,7 @@ def test_copy_layer(tmp_path, testfile, dimensions, suffix_input, suffix_output)
     # Now compare source and dst file
     src_layerinfo = gfo.get_layerinfo(src, raise_on_nogeom=raise_on_nogeom)
     dst_layerinfo = gfo.get_layerinfo(dst, raise_on_nogeom=raise_on_nogeom)
+    assert dst_layerinfo.name == dst.stem
     assert src_layerinfo.featurecount == dst_layerinfo.featurecount
     assert len(src_layerinfo.columns) == len(dst_layerinfo.columns)
     if not (
@@ -409,6 +405,21 @@ def test_copy_layer_input_open_options(tmp_path):
     assert result_gdf.geometry[0].y == 50.939972761
 
 
+@pytest.mark.parametrize("layer", [None, "parcels_output"])
+def test_copy_layer_layer(tmp_path, layer):
+    # Prepare test data
+    src = test_helper.get_testfile("polygon-parcel")
+    dst = tmp_path / "output.gpkg"
+    expected_layer = dst.stem if layer is None else layer
+
+    gfo.copy_layer(src, dst, dst_layer=layer)
+
+    # Check result
+    dst_info = gfo.get_layerinfo(dst)
+    assert dst_info.name == expected_layer
+    assert dst_info.featurecount == 48
+
+
 @pytest.mark.parametrize(
     "src_suffix, dst_suffix, preserve_fid, exp_preserved_fids",
     [