fix(arrow): raise when the Arrow or Parquet output file cannot be opened

src/nyx/output_writers.cpp

@@ -7,6 +7,7 @@
 #ifdef USE_ARROW
 
 #include <iostream>
+#include <stdexcept>
 #include <parquet/arrow/reader.h>
 #include "helpers/fsystem.h"
 #include "helpers/helpers.h"
@@ -32,7 +33,7 @@ arrow::Status ParquetWriter::setup(const std::vector<std::string>& header) {
 
     schema_ = arrow::schema(fields);
 
-    PARQUET_ASSIGN_OR_THROW(
+    ARROW_ASSIGN_OR_RAISE(
         output_stream_, arrow::io::FileOutputStream::Open(output_file_)
     );
 
@@ -54,14 +55,15 @@ arrow::Status ParquetWriter::setup(const std::vector<std::string>& header) {
 
 
 
+// A writer whose setup failed has no usable output stream or file writer, so the
+// constructor throws rather than return a half-built object; WriterFactory::create_writer
+// turns the exception into its error message.
 ParquetWriter::ParquetWriter(const std::string& output_file, const std::vector<std::string>& header) : output_file_(output_file) {
 
     auto status = this->setup(header);
 
-    if (!status.ok()) {
-        // Handle read error
-        std::cout << "Error writing setting up Arrow writer: " << status.ToString() << std::endl;
-    }
+    if (!status.ok())
+        throw std::runtime_error("cannot set up the Parquet writer for " + output_file_ + ": " + status.ToString());
 }
 
 arrow::Status ParquetWriter::write (const std::vector<FTABLE_RECORD>& features) {
@@ -302,15 +304,19 @@ arrow::Status ArrowIPCWriter::setup(const std::vector<std::string>& header) {
 
     writer_ = arrow::ipc::MakeFileWriter(output_stream_, schema_);
 
-    return arrow::Status::OK();
+    return writer_.status();
 }
 
 
 
+// Throws on a failed setup for the same reason as ParquetWriter's constructor: write() and
+// close() dereference writer_, and an unassigned arrow::Result aborts the process there.
 ArrowIPCWriter::ArrowIPCWriter(const std::string& output_file, const std::vector<std::string>& header) : output_file_(output_file) {
 
     auto status = this->setup(header);
 
+    if (!status.ok())
+        throw std::runtime_error("cannot set up the Arrow IPC writer for " + output_file_ + ": " + status.ToString());
 }
 
 arrow::Status ArrowIPCWriter::write (const std::vector<FTABLE_RECORD>& features)
@@ -531,32 +537,40 @@ arrow::Status ArrowIPCWriter::close() {
 
 std::tuple<std::unique_ptr<ApacheArrowWriter>, std::optional<std::string>> WriterFactory::create_writer(const std::string& output_file, const std::vector<std::string>& header) {
 
-    if (Nyxus::ends_with_substr(output_file, ".parquet")) {
+    // the writers' constructors throw when the file cannot be opened for writing
+    // (a directory, a read-only location, a file another process holds mapped)
+    try {
+
+        if (Nyxus::ends_with_substr(output_file, ".parquet")) {
+
+            return { std::make_unique<ParquetWriter>(output_file, header), std::nullopt };
+
+        }
+        else if (Nyxus::ends_with_substr(output_file, ".arrow") || Nyxus::ends_with_substr(output_file, ".feather")) {
 
-        return { std::make_unique<ParquetWriter>(output_file, header), std::nullopt };
+            return { std::make_unique<ArrowIPCWriter>(output_file, header), std::nullopt };
 
+        }
     }
-    else if (Nyxus::ends_with_substr(output_file, ".arrow") || Nyxus::ends_with_substr(output_file, ".feather")) {
+    catch (const std::exception& e) {
 
-        return { std::make_unique<ArrowIPCWriter>(output_file, header), std::nullopt };
+        return { nullptr, std::string{ e.what() } };
 
     }
-    else {
 
-        fs::path path(output_file);
+    fs::path path(output_file);
 
-        auto error_msg = [&path]() {
-            if (path.has_extension())
-            {
-                return "No writer option for extension \"" + path.extension().string() + "\". Valid options are \".parquet\" or \".arrow\".";
-            }
-            else {
-                return std::string{ "No extension type was provided in the path." };
-            }
-        };
+    auto error_msg = [&path]() {
+        if (path.has_extension())
+        {
+            return "No writer option for extension \"" + path.extension().string() + "\". Valid options are \".parquet\" or \".arrow\".";
+        }
+        else {
+            return std::string{ "No extension type was provided in the path." };
+        }
+    };
 
-        return { nullptr, error_msg() };
-    }
+    return { nullptr, error_msg() };
 }
 #endif
 

tests/python/test_nyxus.py

@@ -723,9 +723,66 @@ def test_arrow_ipc_path(self):
 
             arrow_path = nyx.featurize(intens, seg, output_type="arrowipc")
 
-            assert arrow_path == 'NyxusFeatures.arrow'     
+            assert arrow_path == 'NyxusFeatures.arrow'
+
+        @pytest.mark.arrow
+        @pytest.mark.parametrize("output_type, ext", [("arrowipc", ".arrow"), ("parquet", ".parquet")])
+        def test_arrow_unopenable_path_raises(self, tmp_path, output_type, ext):
+            '''
+            An output file that cannot be opened for writing raises RuntimeError naming the
+            file, rather than aborting the interpreter. The target here is a directory, which
+            no platform opens for writing. The same Nyxus object then writes to a writable path.
+            '''
+
+            nyx = nyxus.Nyxus (["MEAN"])
+
+            # a directory output_path resolves to <dir>/NyxusFeatures<ext>
+            blocked = tmp_path/("NyxusFeatures" + ext)
+            blocked.mkdir()
+
+            with pytest.raises(RuntimeError, match="NyxusFeatures" + ext.replace(".", r"\.")):
+                nyx.featurize(intens, seg, output_type=output_type, output_path=str(tmp_path))
+
+            writable = tmp_path/("writable" + ext)
+            out_path = nyx.featurize(intens, seg, output_type=output_type, output_path=str(writable))
+            assert out_path == str(writable)
+            assert writable.is_file()
+
+            if output_type == "parquet":
+                tbl = pq.read_table(out_path)
+            else:
+                with pa.memory_map(out_path, 'rb') as source:
+                    tbl = pa.ipc.open_file(source).read_all()
+            assert tbl.num_rows == len(nyx.featurize(intens, seg))
+
+        @pytest.mark.arrow
+        @pytest.mark.skipif(sys.platform != "win32", reason="only Windows refuses to truncate a memory-mapped file")
+        def test_arrow_ipc_mapped_file_raises(self, tmp_path):
+            '''
+            Featurizing to an Arrow IPC file that a live pyarrow table still maps fails to open
+            on Windows; that failure raises RuntimeError. Once the table is released, the same
+            path is written again.
+            '''
+
+            nyx = nyxus.Nyxus (["MEAN"])
+            out_file = tmp_path/"mapped.arrow"
+
+            arrow_path = nyx.featurize(intens, seg, output_type="arrowipc", output_path=str(out_file))
+            source = pa.memory_map(arrow_path, 'rb')
+            tbl = pa.ipc.open_file(source).read_all()
+            source.close()
+
+            with pytest.raises(RuntimeError, match=r"mapped\.arrow"):
+                nyx.featurize(intens, seg, output_type="arrowipc", output_path=str(out_file))
+
+            n_rows = tbl.num_rows
+            del tbl
+
+            assert nyx.featurize(intens, seg, output_type="arrowipc", output_path=str(out_file)) == str(out_file)
+            with pa.memory_map(arrow_path, 'rb') as source:
+                assert pa.ipc.open_file(source).read_all().num_rows == n_rows
+
 
-
         def test_image_quality(self):
             '''
             Test that ImageQuality class can be imported and will calculated features.

tests/test_all.cc

@@ -884,6 +884,14 @@ TEST(TEST_NYXUS, TEST_ARROW_PARQUET_MECHANICS) {
 	test_arrow_parquet_mechanics();
 }
 
+TEST(TEST_NYXUS, TEST_ARROW_IPC_UNOPENABLE_PATH_MECHANICS) {
+	test_arrow_ipc_unopenable_path_mechanics();
+}
+
+TEST(TEST_NYXUS, TEST_ARROW_PARQUET_UNOPENABLE_PATH_MECHANICS) {
+	test_arrow_parquet_unopenable_path_mechanics();
+}
+
 #endif
 
 //***** 3D shape *****

tests/test_arrow_mechanics.h

@@ -320,3 +320,54 @@ void test_arrow_parquet_mechanics() {
     }
 }
 
+// A target that exists as a directory cannot be opened for writing on any platform, so
+// create_arrow_file must report the failure (naming the path) rather than hand back a writer
+// whose first write aborts the process. The same stream then writes a file next to it, so
+// the refusal is specific to the unopenable path.
+void assert_arrow_unopenable_path_refused(Nyxus::SaveOption saveOption, const std::string& ext)
+{
+    auto temp = fs::temp_directory_path() / ("nyxus_unopenable" + ext);
+    fs::remove_all(temp);
+    fs::create_directories(temp);
+
+    fs::path blocked = temp / ("NyxusFeatures" + ext);
+    fs::create_directory(blocked);
+
+    auto arrow_stream = ArrowOutputStream();
+
+    auto [status, msg] = arrow_stream.create_arrow_file(saveOption, blocked.string(), std::get<0>(features888));
+    ASSERT_FALSE(status) << "a writer was created for directory " << blocked.string();
+    ASSERT_TRUE(msg.has_value());
+    ASSERT_NE(msg->find(blocked.string()), std::string::npos) << "message does not name the path: " << *msg;
+
+    auto [wstatus, wmsg] = arrow_stream.write_arrow_file(std::get<1>(features888));
+    ASSERT_FALSE(wstatus);
+
+    std::string writable = (temp / ("writable" + ext)).string();
+
+    auto [status1, msg1] = arrow_stream.create_arrow_file(saveOption, writable, std::get<0>(features888));
+    ASSERT_TRUE(status1) << msg1.value_or("");
+
+    auto [status2, msg2] = arrow_stream.write_arrow_file(std::get<1>(features888));
+    ASSERT_TRUE(status2) << msg2.value_or("");
+
+    auto [status3, msg3] = arrow_stream.close_arrow_file();
+    ASSERT_TRUE(status3) << msg3.value_or("");
+
+    auto table = get_arrow_table(writable);
+    ASSERT_TRUE(table.ok()) << table.status().ToString();
+    ASSERT_EQ((*table)->num_rows(), (int64_t) std::get<1>(features888).size());
+
+    fs::remove_all(temp);
+}
+
+void test_arrow_ipc_unopenable_path_mechanics()
+{
+    assert_arrow_unopenable_path_refused(Nyxus::SaveOption::saveArrowIPC, ".arrow");
+}
+
+void test_arrow_parquet_unopenable_path_mechanics()
+{
+    assert_arrow_unopenable_path_refused(Nyxus::SaveOption::saveParquet, ".parquet");
+}
+

tests/vetting/not_covered.md

@@ -17,7 +17,7 @@ Regenerate by re-deriving both sets; keep this file in step with each reorg wave
 
 ---
 
-## A. Test files no registry row references — 31 files, 105 test functions
+## A. Test files no registry row references — 31 files, 113 test functions
 
 Both totals are the sum of the two tables below, counting each distinct file once and
 skipping any row struck through as closed, so they can be recomputed from the tables.
@@ -30,7 +30,7 @@ These assert no feature value, so they have no `(feature × config × oracle)` r
 | file | functions | why it has no row |
 |---|---:|---|
 | `test_3d_nifti_mechanics.h` | 2 | NIfTI loader geometry / data access |
-| `test_arrow_mechanics.h` | 2 | Arrow + Parquet writer plumbing |
+| `test_arrow_mechanics.h` | 4 | Arrow + Parquet writer plumbing, including an output path that cannot be opened |
 | `test_arrow_file_name_mechanics.h` | 1 | output-file naming rules |
 | `test_2d_glcm_mechanics.h` | 1 | guards the `GLCM_OFFSET` default (a setting, not a value) |
 | `test_2d_radial_mechanics.h` | 3 | contour frame, centre pixel and normalising radius — the wiring the radial goldens depend on. **Uncredited by decision, not by construction:** see the note below the table |
@@ -202,15 +202,15 @@ Not dead, listed to avoid double-counting: `assert_gldzm_matrix_ibsi`,
 `assert_ngldm_matrix_ibsi_mode`, `assert_ngldm_matrix_nonibsi_mode` take no arguments and have no `TEST()`
 of their own, but each is called by a registered case — they are helpers wearing a `test_` prefix.
 
-### B.3 Registered but excluded by a build flag — 13 cases
+### B.3 Registered but excluded by a build flag — 73 cases
 
 Not dead; they run only where the feature is compiled in. A CPU/tiff-only build silently omits them,
 so a green local run is not a green matrix.
 
 | flag | cases |
 |---|---|
-| `USE_ARROW` | `TEST_ARROW_IPC_MECHANICS`, `TEST_ARROW_PARQUET_MECHANICS`, `TEST_ARROW_FILE_NAMING_MECHANICS` |
-| `OMEZARR_SUPPORT` | `TEST_2D_OMEZARR_TILELOADER_{GEOMETRY,CONTENT,MULTITILE,SIGNED_OFFSET_MAP,FLOAT_QUANTIZED_MAP,UNSIGNED_UNAFFECTED}_MECHANICS`, `TEST_2D_OMEZARR_RAW_{GEOMETRY,CONTENT,MULTITILE,SIGNED_SOURCE_DOMAIN,FLOAT_SOURCE_DOMAIN,UINT32_ACCESSOR_CLAMPS_NEGATIVE,UINT32_ACCESSOR_UNSIGNED_EXACT,UINT32_ACCESSOR_NONFINITE}_MECHANICS` |
+| `USE_ARROW` | `TEST_ARROW_IPC_MECHANICS`, `TEST_ARROW_PARQUET_MECHANICS`, `TEST_ARROW_FILE_NAMING_MECHANICS`, `TEST_ARROW_IPC_UNOPENABLE_PATH_MECHANICS`, `TEST_ARROW_PARQUET_UNOPENABLE_PATH_MECHANICS` |
+| `OMEZARR_SUPPORT` | the 61 `TEST_*OMEZARR*_MECHANICS` cases in `test_all.cc`; `TEST_OMEZARR_V3_ZSTD_MECHANICS` also needs `WITH_ZSTD` |
 | `DICOM_SUPPORT` | `TEST_2D_HU_LOADER_DICOM_{U16,I16}_PRESERVE_MECHANICS`, `TEST_2D_HU_LOADER_DICOM_CT_SMALL_{PRESERVE,BASELINE}_MECHANICS`, `TEST_2D_HU_LOADER_DICOM_FRACTIONAL_SLOPE_{ROUNDS_UNDER_PRESERVE_HU,TRUNCATES_WITHOUT_ROUND_OFFSET}_MECHANICS`, `TEST_2D_HU_DICOM_FRACTIONAL_SLOPE_LOAD_PATH_MECHANICS` |
 
 ### B.4 Python tests skipped at runtime
@@ -220,7 +220,8 @@ so a green local run is not a green matrix.
 - `test_2d_morphology_invariant.py`, `test_2d_glcm_pyradiomics.py`, `test_2d_gldm_pyradiomics.py` — skip when the
   canonical ROI cannot be parsed out of `test_data.h`. **A skip here silently removes an oracle
   assertion**; these should fail rather than skip, since the fixture is committed.
-- `test_nyxus.py` — one case is skipped on Python 3.12, one is `skip_ci`.
+- `test_nyxus.py` — one case is skipped on Python 3.12, one is `skip_ci`, and `test_arrow_ipc_mapped_file_raises`
+  runs on Windows only, the one platform that refuses to truncate a memory-mapped file.
 
 ---