chore: Add pre-commit and explicit compiler warnings (#96)

.github/workflows/dev.yaml

@@ -0,0 +1,33 @@
+
+name: Dev
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    branches:
+      - main
+
+permissions:
+  contents: read
+
+jobs:
+  pre-commit:
+    name: "pre-commit"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          persist-credentials: false
+      - uses: actions/setup-python@v4
+      - name: pre-commit (cache)
+        uses: actions/cache@v3
+        with:
+          path: ~/.cache/pre-commit
+          key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
+      - name: pre-commit (--all-files)
+        run: |
+          python -m pip install pre-commit
+          pre-commit run --show-diff-on-failure --color=always --all-files

.pre-commit-config.yaml

@@ -0,0 +1,30 @@
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v2.3.0
+    hooks:
+    - id: check-yaml
+    - id: end-of-file-fixer
+    - id: trailing-whitespace
+  - repo: https://github.com/pocc/pre-commit-hooks
+    rev: v1.3.5
+    hooks:
+      - id: clang-format
+        args: [-i]
+        types_or: [c, c++]
+  - repo: https://github.com/cheshirekow/cmake-format-precommit
+    rev: v0.6.13
+    hooks:
+    - id: cmake-format
+      args: [--in-place]
+  - repo: https://github.com/psf/black
+    rev: 22.3.0
+    hooks:
+    - id: black
+      types_or: [pyi, python]
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.2.5
+    hooks:
+    - id: codespell
+      types_or: [rst, markdown, c, c++]
+      additional_dependencies: [tomli]
+      exclude: "src/geoarrow/ryu/d2s_intrinsics.h"

CMakeLists.txt

@@ -177,19 +177,22 @@ if(TARGET geoarrow)
                              PRIVATE -Wall
                                      -Werror
                                      -Wextra
-                                     -Wno-type-limits
-                                     -Wno-unused-parameter
                                      -Wpedantic
-                                     -Wunused-result)
+                                     -Wno-type-limits
+                                     -Wmaybe-uninitialized
+                                     -Wunused-result
+                                     -Wconversion
+                                     -Wno-sign-conversion)
     elseif(CMAKE_C_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_C_COMPILER_ID STREQUAL
                                                         "Clang")
       target_compile_options(geoarrow
                              PRIVATE -Wall
                                      -Werror
                                      -Wextra
+                                     -Wpedantic
                                      -Wdocumentation
-                                     -Wno-unused-parameter
-                                     -Wshorten-64-to-32)
+                                     -Wconversion
+                                     -Wno-sign-conversion)
     endif()
   endif()
 endif()

python/README.md

@@ -11,7 +11,7 @@ Python bindings for geoarrow are available on PyPI and can be installed with:
 pip install geoarrow-c
 ```
 
-You can install a developement version with:
+You can install a development version with:
 
 ```bash
 python -m pip install "git+https://github.com/geoarrow/geoarrow-c.git#egg=geoarrow-c&subdirectory=python/geoarrow-c"

python/geoarrow-c/src/geoarrow/c/geoarrow_python/geoarrow_python.h

@@ -7,14 +7,15 @@
 #include "geoarrow.h"
 
 static void PyGeoArrowBufferFree(uint8_t* ptr, int64_t size, void* private_data) {
-  // Aquire the GIL? This buffer very well maybe freed from another thread.
+  // Acquire the GIL? This buffer very well maybe freed from another thread.
   PyObject* obj = (PyObject*)private_data;
   Py_DECREF(obj);
 }
 
-static GeoArrowErrorCode GeoArrowBuilderSetPyBuffer(struct GeoArrowBuilder* builder, int64_t i, PyObject* obj,
+static GeoArrowErrorCode GeoArrowBuilderSetPyBuffer(struct GeoArrowBuilder* builder,
+                                                    int64_t i, PyObject* obj,
                                                     const void* ptr, int64_t size) {
-  // Aquire the GIL? Or maybe not since this should never be initialized from antying
+  // Acquire the GIL? Or maybe not since this should never be initialized from antying
   // that isn't a cython <PyObject*> cast.
   GeoArrowBufferView view;
   view.data = (const uint8_t*)ptr;
@@ -32,4 +33,4 @@ static GeoArrowErrorCode GeoArrowBuilderSetPyBuffer(struct GeoArrowBuilder* buil
   return GEOARROW_OK;
 }
 
-#endif
+#endif

python/geoarrow-c/tests/test_import.py

@@ -3,6 +3,7 @@
 # This is mostly a naive sanity check that the native extension can be loaded
 # on platforms where there is no pyarrow.
 
+
 def test_enums():
     assert ga.CoordType.UNKNOWN == 0
     assert ga.CrsType.UNKNOWN == 1

src/geoarrow/array_view.c

@@ -7,8 +7,7 @@
 
 static int32_t kZeroInt32 = 0;
 
-static int GeoArrowArrayViewInitInternal(struct GeoArrowArrayView* array_view,
-                                         struct GeoArrowError* error) {
+static int GeoArrowArrayViewInitInternal(struct GeoArrowArrayView* array_view) {
   switch (array_view->schema_view.geometry_type) {
     case GEOARROW_GEOMETRY_TYPE_POINT:
       array_view->n_offsets = 0;
@@ -82,15 +81,15 @@ static int GeoArrowArrayViewInitInternal(struct GeoArrowArrayView* array_view,
 GeoArrowErrorCode GeoArrowArrayViewInitFromType(struct GeoArrowArrayView* array_view,
                                                 enum GeoArrowType type) {
   NANOARROW_RETURN_NOT_OK(GeoArrowSchemaViewInitFromType(&array_view->schema_view, type));
-  return GeoArrowArrayViewInitInternal(array_view, NULL);
+  return GeoArrowArrayViewInitInternal(array_view);
 }
 
 GeoArrowErrorCode GeoArrowArrayViewInitFromSchema(struct GeoArrowArrayView* array_view,
                                                   const struct ArrowSchema* schema,
                                                   struct GeoArrowError* error) {
   NANOARROW_RETURN_NOT_OK(
       GeoArrowSchemaViewInit(&array_view->schema_view, schema, error));
-  return GeoArrowArrayViewInitInternal(array_view, error);
+  return GeoArrowArrayViewInitInternal(array_view);
 }
 
 static int GeoArrowArrayViewSetArrayInternal(struct GeoArrowArrayView* array_view,
@@ -202,8 +201,7 @@ static int GeoArrowArrayViewSetArrayInternal(struct GeoArrowArrayView* array_vie
 }
 
 static GeoArrowErrorCode GeoArrowArrayViewSetArraySerialized(
-    struct GeoArrowArrayView* array_view, const struct ArrowArray* array,
-    struct GeoArrowError* error) {
+    struct GeoArrowArrayView* array_view, const struct ArrowArray* array) {
   array_view->length[0] = array->length;
   array_view->offset[0] = array->offset;
 
@@ -218,8 +216,7 @@ GeoArrowErrorCode GeoArrowArrayViewSetArray(struct GeoArrowArrayView* array_view
   switch (array_view->schema_view.type) {
     case GEOARROW_TYPE_WKT:
     case GEOARROW_TYPE_WKB:
-      NANOARROW_RETURN_NOT_OK(
-          GeoArrowArrayViewSetArraySerialized(array_view, array, error));
+      NANOARROW_RETURN_NOT_OK(GeoArrowArrayViewSetArraySerialized(array_view, array));
       break;
     default:
       NANOARROW_RETURN_NOT_OK(

src/geoarrow/array_view_test.cc

@@ -848,7 +848,7 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidMultiLinestringWithOffset) {
   ASSERT_EQ(ArrowArrayInitFromSchema(&array, &schema, nullptr), GEOARROW_OK);
   ASSERT_EQ(ArrowArrayStartAppending(&array), GEOARROW_OK);
 
-  // First null wont be used because of offset
+  // First null won't be used because of offset
   ASSERT_EQ(ArrowArrayAppendNull(&array, 2), GEOARROW_OK);
 
   // First ring won't be used because we will set the offset 1
@@ -1032,7 +1032,7 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidMultipolygonWithOffsets) {
   // First NULL won't be used because of offset
   ASSERT_EQ(ArrowArrayAppendNull(&array, 2), GEOARROW_OK);
 
-  // Empty polygon that will be skipped becasue of offset
+  // Empty polygon that will be skipped because of offset
   ASSERT_EQ(ArrowArrayAppendEmpty(array.children[0], 1), GEOARROW_OK);
 
   ASSERT_EQ(ArrowArrayFinishElement(&array), GEOARROW_OK);

src/geoarrow/builder.c

@@ -143,7 +143,7 @@ static GeoArrowErrorCode GeoArrowBuilderInitInternal(struct GeoArrowBuilder* bui
     return result;
   }
 
-  // Initalize one empty coordinate for the visitor pattern
+  // Initialize one empty coordinate for the visitor pattern
   memcpy(private->empty_coord_values, kEmptyPointCoords, 4 * sizeof(double));
   private->empty_coord.values[0] = private->empty_coord_values;
   private->empty_coord.values[1] = private->empty_coord_values + 1;
@@ -488,6 +488,8 @@ static int feat_start_point(struct GeoArrowVisitor* v) {
 static int geom_start_point(struct GeoArrowVisitor* v,
                             enum GeoArrowGeometryType geometry_type,
                             enum GeoArrowDimensions dimensions) {
+  NANOARROW_UNUSED(geometry_type);
+
   // level++, geometry type, dimensions, reset size
   // validate dimensions, maybe against some options that indicate
   // error for mismatch, fill, or drop behaviour
@@ -497,7 +499,10 @@ static int geom_start_point(struct GeoArrowVisitor* v,
   return GEOARROW_OK;
 }
 
-static int ring_start_point(struct GeoArrowVisitor* v) { return GEOARROW_OK; }
+static int ring_start_point(struct GeoArrowVisitor* v) {
+  NANOARROW_UNUSED(v);
+  return GEOARROW_OK;
+}
 
 static int coords_point(struct GeoArrowVisitor* v,
                         const struct GeoArrowCoordView* coords) {
@@ -508,9 +513,15 @@ static int coords_point(struct GeoArrowVisitor* v,
                                      coords->n_coords);
 }
 
-static int ring_end_point(struct GeoArrowVisitor* v) { return GEOARROW_OK; }
+static int ring_end_point(struct GeoArrowVisitor* v) {
+  NANOARROW_UNUSED(v);
+  return GEOARROW_OK;
+}
 
-static int geom_end_point(struct GeoArrowVisitor* v) { return GEOARROW_OK; }
+static int geom_end_point(struct GeoArrowVisitor* v) {
+  NANOARROW_UNUSED(v);
+  return GEOARROW_OK;
+}
 
 static int null_feat_point(struct GeoArrowVisitor* v) {
   struct GeoArrowBuilder* builder = (struct GeoArrowBuilder*)v->private_data;