Efficient swc build (#476)

* handles `locale` correctly, so decimal numbers using comma are allowed
* ~800 line SWC parse time goes from 0.976 to 0.699
* Doesn't create changes to bifurcations at the root nodes of SWC files
* More strict Neuromorpho 3 point soma checks disallow repeated Sample IDs
* Follow https://swc-specification.readthedocs.io/en/latest/swc.html more closely
* Allow only positive integer Sample IDs
* Allow for the type of neurite to change at bifurcation points
* Allow Window style `End of Lines`
* allow trailing spaces on sample line

.github/workflows/coverage_test.yaml

@@ -19,14 +19,19 @@ jobs:
         uses: actions/checkout@v4
       - name: Fetch repository
         run: git fetch --prune --unshallow
+
       - name: Get submodules
         run: git submodule update --init --force --recursive
+
       - name: Install packages
         run: |
           sudo apt-get ${{env.apt_options}} update -y
-          sudo apt-get ${{env.apt_options}} install -y build-essential libhdf5-dev lcov
+          sudo apt-get ${{env.apt_options}} install -y build-essential libhdf5-dev lcov language-pack-de
       - name: Build and run unittests
-        run: ci/coverage_test.sh
+        run: |
+          locale -a 
+          ci/coverage_test.sh
+
       - name: Upload Coverage to Coveralls
         uses: coverallsapp/github-action@master
         with:

.github/workflows/run-tests.yml

@@ -37,7 +37,7 @@ jobs:
         if: runner.os == 'Linux'
         run: |
           sudo apt-get ${{env.apt_options}} update -y
-          sudo apt-get ${{env.apt_options}} install -y build-essential libhdf5-dev
+          sudo apt-get ${{env.apt_options}} install -y build-essential libhdf5-dev language-pack-de
 
       - name: Install packages on MacOS
         if: runner.os == 'macOS'
@@ -55,6 +55,7 @@ jobs:
           CC: ${{ matrix.compiler.CC }}
           CXX: ${{ matrix.compiler.CXX }}
         run: |
+          locale -a
           ./ci/python_test.sh
           ./ci/cpp_test.sh
           ./ci/cpp_test.sh "-DMORPHIO_USE_DOUBLE=ON"

CHANGELOG.md

@@ -1,3 +1,33 @@
+v3.4.0
+======
+Improvements:
+* New SWC parser:
+    * handles `locale` correctly, so decimal numbers using comma are allowed
+    * ~800 line SWC parse time goes from 0.976 to 0.699
+    * Doesn't create changes to bifurcations at the root nodes of SWC files
+    * More strict Neuromorpho 3 point soma checks
+    * disallow repeated Sample IDs
+    * Follow https://swc-specification.readthedocs.io/en/latest/swc.html more closely
+    * Allow only positive integer Sample IDs 
+    * Allow for the type of neurite to change at bifurcation points
+    * Allow Window style `End of Lines`
+    * allow trailing spaces on sample lines
+
+* Since this is a large change to the SWC parser, the minor release was increased 
+
+v3.3.10
+=======
+* This is primarily so that 3.13 wheels exist before the new SWC parser is put into production.
+* try to work around removal of older MSVC tools
+* Remove Errorlevel::Info; it's not an error if it's only informative
+* Update upload/download actions to v4.
+* update HDF5 url
+* Update HighFive to 2.10.0
+
+v3.3.9
+======
+* remove embarrassing debug print; Hat-tip SarahM
+
 v3.3.8
 ======
 * Make ThreePointSoma use relative tolerance (494)

binds/python/bind_misc.cpp

@@ -283,5 +283,4 @@ Note: This API is 'experimental', meaning it might change in the future.
             // Bind the lifetime of the `morphio::LoadUnordered` (1) to the
             // lifetime of the returned iterator (0).
             py::keep_alive<0, 1>());
-
 }

binds/python/bind_warnings_exceptions.cpp

@@ -104,7 +104,7 @@ void bind_warnings_exceptions(py::module& m) {
     (void) C(WriteUndefinedSoma);
     (void) C(MitochondriaWriteNotSupported);
     (void) C(SomaNonContour);
-    (void) C(SomaNonCynlinderOrPoint);
+    (void) C(SomaNonCylinderOrPoint);
 #undef QUOTE
 #undef C
 }

binds/python/generated/docstrings.h

@@ -862,15 +862,15 @@ static const char *mkd_doc_morphio_SomaNonContour_msg = R"doc()doc";
 
 static const char *mkd_doc_morphio_SomaNonContour_warning = R"doc()doc";
 
-static const char *mkd_doc_morphio_SomaNonCynlinderOrPoint = R"doc()doc";
+static const char *mkd_doc_morphio_SomaNonCylinderOrPoint = R"doc()doc";
 
-static const char *mkd_doc_morphio_SomaNonCynlinderOrPoint_SomaNonCynlinderOrPoint = R"doc()doc";
+static const char *mkd_doc_morphio_SomaNonCylinderOrPoint_SomaNonCylinderOrPoint = R"doc()doc";
 
-static const char *mkd_doc_morphio_SomaNonCynlinderOrPoint_errorLevel = R"doc()doc";
+static const char *mkd_doc_morphio_SomaNonCylinderOrPoint_errorLevel = R"doc()doc";
 
-static const char *mkd_doc_morphio_SomaNonCynlinderOrPoint_msg = R"doc()doc";
+static const char *mkd_doc_morphio_SomaNonCylinderOrPoint_msg = R"doc()doc";
 
-static const char *mkd_doc_morphio_SomaNonCynlinderOrPoint_warning = R"doc()doc";
+static const char *mkd_doc_morphio_SomaNonCylinderOrPoint_warning = R"doc()doc";
 
 static const char *mkd_doc_morphio_Soma_Soma = R"doc()doc";
 

include/morphio/warning_handling.h

@@ -243,8 +243,8 @@ struct SomaNonContour: public WarningMessage {
     }
 };
 
-struct SomaNonCynlinderOrPoint: public WarningMessage {
-    SomaNonCynlinderOrPoint()
+struct SomaNonCylinderOrPoint: public WarningMessage {
+    SomaNonCylinderOrPoint()
         : WarningMessage(std::string()) {}
     Warning warning() const final {
         return Warning::SOMA_NON_CYLINDER_OR_POINT;

src/CMakeLists.txt

@@ -28,6 +28,7 @@ set(MORPHIO_SOURCES
     readers/morphologyASC.cpp
     readers/morphologyHDF5.cpp
     readers/morphologySWC.cpp
+    readers/utils.cpp
     readers/vasculatureHDF5.cpp
     section.cpp
     shared_utils.cpp

src/enums.cpp

@@ -8,6 +8,7 @@
 namespace morphio {
 namespace enums {
 
+/** Output stream formatter for SomaType */
 std::ostream& operator<<(std::ostream& os, const SomaType v) {
     switch (v) {
     case SOMA_SINGLE_POINT:

src/error_message_generation.cpp

@@ -13,7 +13,7 @@ std::string errorMsg(const std::string& uri,
                      const std::string& msg) {
     return "\n" + (uri.empty() ? "" : errorLink(uri, lineNumber, errorLevel) + "\n") + msg;
 }
-} // namespace
+}  // namespace
 
 // LCOV_EXCL_START {  all the error messages are excluded from coverage
 
@@ -82,8 +82,14 @@ std::string ErrorMessages::ERROR_REPEATED_ID(const unsigned int originalId,
                     newLineNumber,
                     ErrorLevel::WARNING,
                     "Repeated ID: " + std::to_string(originalId)) +
-           "\nID already appears here: \n" +
-           errorLink(_uri, originalLineNumber, ErrorLevel::WARNING);
+           "\nID already appears here: \n" + errorLink(_uri, originalLineNumber, ErrorLevel::WARNING);
+}
+
+std::string ErrorMessages::EARLY_END_OF_FILE(long unsigned int lineNumber) const {
+    return errorMsg(_uri,
+                    lineNumber,
+                    ErrorLevel::ERROR,
+                    "The end of the file was reached before parsing finshed");
 }
 
 std::string ErrorMessages::ERROR_SELF_PARENT(const unsigned int lineNumber) const {
@@ -100,6 +106,13 @@ std::string ErrorMessages::ERROR_MISSING_MITO_PARENT(int mitoParentId) const {
            std::to_string(mitoParentId) + " does not exist.";
 }
 
+std::string ErrorMessages::ERROR_NEGATIVE_ID(long unsigned int lineNumber) const {
+    return errorMsg(_uri,
+                    lineNumber,
+                    ErrorLevel::WARNING,
+                    "The ID assigned to this line is negative");
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //             NEUROLUCIDA
 ////////////////////////////////////////////////////////////////////////////////
@@ -157,6 +170,9 @@ std::string ErrorMessages::ERROR_UNCOMPATIBLE_FLAGS(morphio::Option flag1,
 ////////////////////////////////////////////////////////////////////////////////
 //              WRITERS
 ////////////////////////////////////////////////////////////////////////////////
+std::string ErrorMessages::ERROR_EMPTY_MORPHOLOGY() const {
+    return errorMsg(_uri, 0, ErrorLevel::ERROR, "Morphology is empty.");
+}
 
 std::string ErrorMessages::ERROR_UNSUPPORTED_SECTION_TYPE(const SectionType& type) const {
     return ("Attempted to write unsupported section type: " + std::to_string(type) +

src/error_message_generation.h

@@ -52,9 +52,7 @@ class ErrorMessages
     std::string ERROR_MULTIPLE_SOMATA(const std::vector<unsigned int>& lineNumbers) const;
 
     /** Missing section parent error message */
-    std::string ERROR_MISSING_PARENT(unsigned int id,
-                                     int parentId,
-                                     unsigned int lineNumber) const;
+    std::string ERROR_MISSING_PARENT(unsigned int id, int parentId, unsigned int lineNumber) const;
 
     /** Bifurcating soma error message */
     std::string ERROR_SOMA_BIFURCATION(unsigned int sampleLineNumber,
@@ -71,12 +69,21 @@ class ErrorMessages
     /** Section self parent error message */
     std::string ERROR_SELF_PARENT(unsigned int lineNumber) const;
 
+    /** The end of the file was reached before parsing finshed */
+    std::string EARLY_END_OF_FILE(long unsigned int lineNumber) const;
+
     /** Undefined soma error message */
     std::string ERROR_NOT_IMPLEMENTED_UNDEFINED_SOMA(const std::string&) const;
 
     /** Missing mitochondria parent section error message */
     std::string ERROR_MISSING_MITO_PARENT(int mitoParentId) const;
 
+    ////////////////////////////////////////////////////////////////////////////////
+    //             SWC
+    ////////////////////////////////////////////////////////////////////////////////
+    /** A negative ID is used in SWC */
+    std::string ERROR_NEGATIVE_ID(long unsigned int lineNumber) const;
+
     ////////////////////////////////////////////////////////////////////////////////
     //             NEUROLUCIDA
     ////////////////////////////////////////////////////////////////////////////////
@@ -111,6 +118,9 @@ class ErrorMessages
     //              WRITERS
     ////////////////////////////////////////////////////////////////////////////////
 
+    /** Morphology is empty */
+    std::string ERROR_EMPTY_MORPHOLOGY() const;
+
     /** Unsupported morphology section type error message */
     std::string ERROR_UNSUPPORTED_SECTION_TYPE(const enums::SectionType& type) const;
 
@@ -125,6 +135,7 @@ class ErrorMessages
 
     /** Can't write perimeter data to SWC, ASC error message */
     std::string ERROR_PERIMETER_DATA_NOT_WRITABLE();
+
     /** Single section child SWC error message */
     std::string ERROR_ONLY_CHILD_SWC_WRITER(unsigned int parentId) const;
 
@@ -136,6 +147,7 @@ class ErrorMessages
 
     /** Contour soma must have at least 3 points. */
     std::string ERROR_SOMA_INVALID_CONTOUR() const;
+
   private:
     std::string _uri;
 };

src/mut/morphology.cpp

@@ -345,8 +345,7 @@ breadth_iterator Morphology::breadth_end() const {
 void Morphology::applyModifiers(unsigned int modifierFlags) {
     if (modifierFlags & NO_DUPLICATES & TWO_POINTS_SECTIONS) {
         const auto err = details::ErrorMessages(_uri);
-        throw SectionBuilderError(
-            err.ERROR_UNCOMPATIBLE_FLAGS(NO_DUPLICATES, TWO_POINTS_SECTIONS));
+        throw SectionBuilderError(err.ERROR_UNCOMPATIBLE_FLAGS(NO_DUPLICATES, TWO_POINTS_SECTIONS));
     }
 
     if (modifierFlags & SOMA_SPHERE) {
@@ -389,12 +388,6 @@ std::unordered_map<int, std::vector<unsigned int>> Morphology::connectivity() {
 }
 
 void Morphology::write(const std::string& filename) const {
-    for (const auto& root : rootSections()) {
-        if (root->points().size() < 2) {
-            throw morphio::SectionBuilderError("Root sections must have at least 2 points");
-        }
-    }
-
     const size_t pos = filename.find_last_of('.');
     if (pos == std::string::npos) {
         throw UnknownFileType("Missing file extension.");

src/mut/writer_asc.cpp

@@ -54,13 +54,14 @@ void asc(const Morphology& morph,
          const std::string& filename,
          std::shared_ptr<morphio::WarningHandler> handler) {
     if (details::emptyMorphology(morph, handler)) {
-        return;
+        throw morphio::WriterError(morphio::details::ErrorMessages().ERROR_EMPTY_MORPHOLOGY());
     }
 
     details::validateContourSoma(morph, handler);
     details::checkSomaHasSameNumberPointsDiameters(*morph.soma());
     details::validateHasNoMitochondria(morph, handler);
     details::validateHasNoPerimeterData(morph);
+    details::validateRootPointsHaveTwoOrMorePoints(morph);
 
     std::ofstream myfile(filename);
 
@@ -80,7 +81,8 @@ void asc(const Morphology& morph,
         } else if (type == SECTION_APICAL_DENDRITE) {
             myfile << "( (Color Red)\n  (Apical)\n";
         } else {
-            throw WriterError(morphio::details::ErrorMessages().ERROR_UNSUPPORTED_SECTION_TYPE(type));
+            throw WriterError(
+                morphio::details::ErrorMessages().ERROR_UNSUPPORTED_SECTION_TYPE(type));
         }
         write_asc_section(myfile, section, 2);
         myfile << ")\n\n";

src/mut/writer_hdf5.cpp

@@ -13,8 +13,9 @@
 #include <highfive/H5File.hpp>
 #include <highfive/H5Object.hpp>
 
-#include "../shared_utils.hpp"
 #include "../error_message_generation.h"
+#include "../shared_utils.hpp"
+#include "morphio/exceptions.h"
 #include "writer_utils.h"
 
 namespace {
@@ -151,11 +152,12 @@ void h5(const Morphology& morph,
         const std::string& filename,
         std::shared_ptr<morphio::WarningHandler> handler) {
     if (details::emptyMorphology(morph, handler)) {
-        return;
+        throw morphio::WriterError(morphio::details::ErrorMessages().ERROR_EMPTY_MORPHOLOGY());
     }
 
     details::validateContourSoma(morph, handler);
     details::checkSomaHasSameNumberPointsDiameters(*morph.soma());
+    details::validateRootPointsHaveTwoOrMorePoints(morph);
 
     HighFive::File h5_file(filename,
                            HighFive::File::ReadWrite | HighFive::File::Create |
@@ -169,7 +171,6 @@ void h5(const Morphology& morph,
 
     const std::vector<Point>& somaPoints = morph.soma()->points();
     const auto& somaDiameters = morph.soma()->diameters();
-
     for (unsigned int i = 0; i < somaPoints.size(); ++i) {
         raw_points.push_back(
             {somaPoints[i][0], somaPoints[i][1], somaPoints[i][2], somaDiameters[i]});

src/mut/writer_swc.cpp

@@ -20,8 +20,8 @@
 #include <highfive/H5File.hpp>
 #include <highfive/H5Object.hpp>
 
-#include "../shared_utils.hpp"
 #include "../error_message_generation.h"
+#include "../shared_utils.hpp"
 #include "writer_utils.h"
 
 namespace {
@@ -122,7 +122,7 @@ void validateSWCSoma(const morphio::mut::Morphology& morph,
     } else if (!(soma->type() == SomaType::SOMA_NEUROMORPHO_THREE_POINT_CYLINDERS ||
                  soma->type() == SomaType::SOMA_CYLINDERS ||
                  soma->type() == SomaType::SOMA_SINGLE_POINT)) {
-        handler->emit(std::make_shared<morphio::SomaNonCynlinderOrPoint>());
+        handler->emit(std::make_shared<morphio::SomaNonCylinderOrPoint>());
     } else if (soma->type() == SomaType::SOMA_SINGLE_POINT && soma_points.size() != 1) {
         throw WriterError(ErrorMessages().ERROR_SOMA_INVALID_SINGLE_POINT());
     } else if (soma->type() == SomaType::SOMA_NEUROMORPHO_THREE_POINT_CYLINDERS &&
@@ -142,7 +142,8 @@ void swc(const Morphology& morph,
          const std::string& filename,
          std::shared_ptr<morphio::WarningHandler> handler) {
     if (details::emptyMorphology(morph, handler)) {
-        return;
+        throw morphio::WriterError(
+            morphio::details::ErrorMessages(filename).ERROR_EMPTY_MORPHOLOGY());
     }
 
     const std::shared_ptr<Soma>& soma = morph.soma();

src/mut/writer_utils.cpp

@@ -71,6 +71,15 @@ void validateHasNoMitochondria(const morphio::mut::Morphology& morph,
     }
 }
 
+void validateRootPointsHaveTwoOrMorePoints(const morphio::mut::Morphology& morph) {
+    for (const auto& root : morph.rootSections()) {
+        if (root->points().size() < 2) {
+            throw morphio::RawDataError("Root sections must have at least 2 points");
+        }
+    }
+
+}
+
 }  // namespace details
 }  // namespace writer
 }  // namespace mut

src/mut/writer_utils.h

@@ -22,6 +22,7 @@ void validateContourSoma(const morphio::mut::Morphology&,
 void validateHasNoPerimeterData(const morphio::mut::Morphology&);
 void validateHasNoMitochondria(const morphio::mut::Morphology&,
                                std::shared_ptr<morphio::WarningHandler> handler);
+void validateRootPointsHaveTwoOrMorePoints(const morphio::mut::Morphology& morph);
 
 }  // namespace details
 }  // namespace writer