From c77f70ff6a2d6e545ab8a8e568920db85e5c2273 Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Fri, 5 Jan 2024 15:30:00 +0100 Subject: [PATCH] plumb into collections, various cleanups --- binds/python/bind_misc.cpp | 26 ++++--- binds/python/generated/docstrings.h | 8 +-- binds/python/morphio.cpp | 2 +- include/morphio/collection.h | 34 +++++++--- include/morphio/errorMessages.h | 2 +- include/morphio/morphology.h | 8 +-- include/morphio/mut/morphology.h | 33 ++++----- src/collection.cpp | 102 ++++++++++++++++++---------- src/errorMessages.cpp | 12 ++-- src/error_message_generation.cpp | 9 --- src/morphology.cpp | 10 +-- src/mut/morphology.cpp | 23 ++++--- src/mut/section.cpp | 2 +- src/readers/morphologyHDF5.cpp | 7 +- src/readers/morphologyHDF5.h | 2 +- src/readers/morphologySWC.cpp | 11 ++- tests/test_11_collection.py | 7 +- tests/test_1_swc.py | 2 +- tests/test_4_immut.py | 5 +- tests/test_immutable_morphology.cpp | 8 +-- 20 files changed, 180 insertions(+), 133 deletions(-) diff --git a/binds/python/bind_misc.cpp b/binds/python/bind_misc.cpp index 7de4ef052..d442917fb 100644 --- a/binds/python/bind_misc.cpp +++ b/binds/python/bind_misc.cpp @@ -182,36 +182,42 @@ void bind_misc(py::module& m) { [](morphio::Collection* collection, const std::string& morph_name, unsigned int options, - bool is_mutable) -> py::object { + bool is_mutable, + std::shared_ptr warning_handler) -> py::object { if (is_mutable) { - return py::cast( - collection->load(morph_name, options)); + return py::cast(collection->load(morph_name, + options, + warning_handler)); } else { - return py::cast(collection->load(morph_name, options)); + return py::cast(collection->load(morph_name, + options, + warning_handler)); } }, "morph_name"_a, "options"_a = morphio::enums::Option::NO_MODIFIER, "mutable"_a = false, + "warning_handler"_a = std::shared_ptr(nullptr), "Load the morphology named 'morph_name' form the collection.") .def( "load_unordered", [](morphio::Collection* collection, std::vector morphology_names, unsigned int options, - bool is_mutable) -> py::object { + bool is_mutable, + std::shared_ptr warning_handler) -> py::object { if (is_mutable) { - return py::cast( - collection->load_unordered(morphology_names, - options)); + return py::cast(collection->load_unordered( + morphology_names, options, warning_handler)); } else { - return py::cast( - collection->load_unordered(morphology_names, options)); + return py::cast(collection->load_unordered( + morphology_names, options, warning_handler)); } }, "morphology_names"_a, "options"_a = morphio::enums::Option::NO_MODIFIER, "mutable"_a = false, + "warning_handler"_a = std::shared_ptr(nullptr), R"(Create an iterable of loop index and morphology. When reading from containers, the order in which morphologies are read can diff --git a/binds/python/generated/docstrings.h b/binds/python/generated/docstrings.h index 3eb39b240..f045dd7d0 100644 --- a/binds/python/generated/docstrings.h +++ b/binds/python/generated/docstrings.h @@ -1243,10 +1243,10 @@ static const char *mkd_doc_morphio_enums_operator_lshift = R"doc()doc"; static const char *mkd_doc_morphio_get = R"doc()doc"; -static const char *mkd_doc_morphio_getErrorHandler = R"doc()doc"; - static const char *mkd_doc_morphio_getVersionString = R"doc()doc"; +static const char *mkd_doc_morphio_getWarningHandler = R"doc()doc"; + static const char *mkd_doc_morphio_isRoot = R"doc(Return true if this section is a root section (parent ID == -1))doc"; static const char *mkd_doc_morphio_mut_DendriticSpine = R"doc(Mutable(editable) morphio::DendriticSpine)doc"; @@ -1528,7 +1528,7 @@ static const char *mkd_doc_morphio_mut_Morphology_endoplasmicReticulum_3 = R"doc static const char *mkd_doc_morphio_mut_Morphology_eraseByValue = R"doc()doc"; -static const char *mkd_doc_morphio_mut_Morphology_getHandler = R"doc()doc"; +static const char *mkd_doc_morphio_mut_Morphology_getWarningHandler = R"doc()doc"; static const char *mkd_doc_morphio_mut_Morphology_handler = R"doc()doc"; @@ -1561,8 +1561,6 @@ static const char *mkd_doc_morphio_mut_Morphology_sections = R"doc(Returns the d static const char *mkd_doc_morphio_mut_Morphology_sections_2 = R"doc()doc"; -static const char *mkd_doc_morphio_mut_Morphology_setErrorHandler = R"doc()doc"; - static const char *mkd_doc_morphio_mut_Morphology_soma = R"doc(Returns a shared pointer on the Soma diff --git a/binds/python/morphio.cpp b/binds/python/morphio.cpp index 229d24b10..47634a75a 100644 --- a/binds/python/morphio.cpp +++ b/binds/python/morphio.cpp @@ -22,8 +22,8 @@ namespace py = pybind11; PYBIND11_MODULE(_morphio, m) { bind_enums(m); - bind_misc(m); bind_warnings_exceptions(m); + bind_misc(m); bind_immutable(m); diff --git a/include/morphio/collection.h b/include/morphio/collection.h index 4026161c3..847b38824 100644 --- a/include/morphio/collection.h +++ b/include/morphio/collection.h @@ -53,15 +53,19 @@ class Collection * Load the morphology as an immutable morphology. */ template - typename enable_if_mutable::type load(const std::string& morph_name, - unsigned int options = NO_MODIFIER) const; + typename enable_if_mutable::type load( + const std::string& morph_name, + unsigned int options = NO_MODIFIER, + std::shared_ptr warning_handler = nullptr) const; /** * Load the morphology as a mutable morphology. */ template - typename enable_if_immutable::type load(const std::string& morph_name, - unsigned int options = NO_MODIFIER) const; + typename enable_if_immutable::type load( + const std::string& morph_name, + unsigned int options = NO_MODIFIER, + std::shared_ptr warning_handler = nullptr) const; /** * Returns an iterable of loop index, morphology pairs. @@ -69,8 +73,10 @@ class Collection * See `LoadUnordered` for details. */ template - LoadUnordered load_unordered(std::vector morphology_names, - unsigned int options = NO_MODIFIER) const; + LoadUnordered load_unordered( + std::vector morphology_names, + unsigned int options = NO_MODIFIER, + std::shared_ptr warning_handler = nullptr) const; /** * Returns the reordered loop indices. @@ -183,15 +189,23 @@ extern template LoadUnordered::Iterator::operator*() const; extern template typename enable_if_mutable::type -Collection::load(const std::string& morph_name, unsigned int options) const; +Collection::load(const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const; extern template typename enable_if_immutable::type -Collection::load(const std::string& morph_name, unsigned int options) const; +Collection::load(const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const; extern template LoadUnordered Collection::load_unordered( - std::vector morphology_names, unsigned int options) const; + std::vector morphology_names, + unsigned int options, + std::shared_ptr warning_handler) const; extern template LoadUnordered Collection::load_unordered( - std::vector morphology_names, unsigned int options) const; + std::vector morphology_names, + unsigned int options, + std::shared_ptr warning_handler) const; } // namespace morphio diff --git a/include/morphio/errorMessages.h b/include/morphio/errorMessages.h index 4731cf00f..970e08450 100644 --- a/include/morphio/errorMessages.h +++ b/include/morphio/errorMessages.h @@ -21,6 +21,6 @@ void set_ignored_warning(enums::Warning warning, bool ignore = true); /** Set an array of warnings to ignore **/ void set_ignored_warning(const std::vector& warning, bool ignore = true); -std::shared_ptr getErrorHandler(); +std::shared_ptr getWarningHandler(); } // namespace morphio diff --git a/include/morphio/morphology.h b/include/morphio/morphology.h index 15b5998b5..81efb25c4 100644 --- a/include/morphio/morphology.h +++ b/include/morphio/morphology.h @@ -48,13 +48,11 @@ class Morphology unsigned int options = NO_MODIFIER, std::shared_ptr = nullptr); - /** Constructor from an already parsed file */ - explicit Morphology(const HighFive::Group& group, unsigned int options = NO_MODIFIER); /** Constructor from an already parsed file */ - // explicit Morphology(const HighFive::Group& group, - // unsigned int options = NO_MODIFIER, - // std::shared_ptr = nullptr); + explicit Morphology(const HighFive::Group& group, + unsigned int options = NO_MODIFIER, + std::shared_ptr = nullptr); /** Constructor from an instance of morphio::mut::Morphology */ explicit Morphology(const mut::Morphology&); diff --git a/include/morphio/mut/morphology.h b/include/morphio/mut/morphology.h index 0cd905e57..9a315713e 100644 --- a/include/morphio/mut/morphology.h +++ b/include/morphio/mut/morphology.h @@ -32,11 +32,10 @@ bool _checkDuplicatePoint(const std::shared_ptr
& parent, class Morphology { public: - Morphology() + Morphology(std::shared_ptr warning_handler = nullptr) : _soma(std::make_shared()) - , _cellProperties( - std::make_shared(morphio::Property::CellLevel())) - , _handler(getErrorHandler()) {} + , _cellProperties(std::make_shared()) + , _handler(warning_handler ? warning_handler : morphio::getWarningHandler()) {} /** Build a mutable Morphology from an on-disk morphology @@ -52,10 +51,12 @@ class Morphology std::shared_ptr warning_handler = nullptr); /// Build a mutable Morphology from an HighFive::Group - explicit Morphology(const HighFive::Group& group, unsigned int options = NO_MODIFIER); + explicit Morphology(const HighFive::Group& group, + unsigned int options = NO_MODIFIER, + std::shared_ptr warning_handler = nullptr); /// Build a mutable Morphology from a mutable morphology - Morphology(const morphio::mut::Morphology& morphology, + Morphology(const mut::Morphology& morphology, unsigned int options = NO_MODIFIER, std::shared_ptr warning_handler = nullptr); @@ -199,11 +200,11 @@ class Morphology /// Write file to H5, SWC, ASC format depending on filename extension void write(const std::string& filename) const; - void addAnnotation(const morphio::Property::Annotation& annotation) { + void addAnnotation(const Property::Annotation& annotation) { _cellProperties->_annotations.push_back(annotation); } - void addMarker(const morphio::Property::Marker& marker) { + void addMarker(const Property::Marker& marker) { _cellProperties->_markers.push_back(marker); } @@ -223,18 +224,14 @@ class Morphology **/ void removeUnifurcations(); - std::shared_ptr getHandler() const { + std::shared_ptr getWarningHandler() const { return _handler; } - void setErrorHandler(std::shared_ptr h) { - _handler = h; - } - std::shared_ptr _soma; - std::shared_ptr _cellProperties; + std::shared_ptr _cellProperties; EndoplasmicReticulum _endoplasmicReticulum; - morphio::Property::DendriticSpine::Level _dendriticSpineLevel; + Property::DendriticSpine::Level _dendriticSpineLevel; private: std::vector> _rootSections; @@ -256,10 +253,8 @@ class Morphology std::string _uri; friend class Section; - friend void modifiers::nrn_order(morphio::mut::Morphology& morpho); - friend bool diff(const Morphology& left, - const Morphology& right, - morphio::enums::LogLevel verbose); + friend void modifiers::nrn_order(mut::Morphology& morpho); + friend bool diff(const Morphology& left, const Morphology& right, enums::LogLevel verbose); }; } // namespace mut diff --git a/src/collection.cpp b/src/collection.cpp index a4c4354dc..24e263559 100644 --- a/src/collection.cpp +++ b/src/collection.cpp @@ -39,11 +39,13 @@ class LoadUnorderedFromLoopIndices: public LoadUnorderedImpl LoadUnorderedFromLoopIndices(Collection collection, std::vector loop_indices, std::vector morphology_names, - unsigned int options) + unsigned int options, + std::shared_ptr warning_handler) : _collection(std::move(collection)) , _loop_indices(std::move(loop_indices)) , _morphology_names(std::move(morphology_names)) - , _options(options) {} + , _options(options) + , _warning_handler(std::move(warning_handler)) {} size_t size() const override { return _morphology_names.size(); @@ -69,6 +71,7 @@ class LoadUnorderedFromLoopIndices: public LoadUnorderedImpl std::vector _loop_indices; std::vector _morphology_names; unsigned int _options; + std::shared_ptr _warning_handler; }; } // namespace detail @@ -79,13 +82,19 @@ class CollectionImpl public: virtual ~CollectionImpl() = default; - virtual Morphology load(const std::string& morph_name, unsigned int options) const = 0; + virtual Morphology load(const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const = 0; - virtual mut::Morphology load_mut(const std::string& morph_name, unsigned int options) const = 0; + virtual mut::Morphology load_mut(const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const = 0; - virtual std::shared_ptr load_unordered(Collection collection, - std::vector morph_name, - unsigned int options) const = 0; + virtual std::shared_ptr load_unordered( + Collection collection, + std::vector morph_name, + unsigned int options, + std::shared_ptr warning_handler) const = 0; virtual std::vector argsort(const std::vector& morphology_names) const = 0; }; @@ -97,25 +106,32 @@ class CollectionImpl: public morphio::CollectionImpl // The purpose of this class is to implement the two separate // functions in terms of a single templated method `load_impl`. public: - morphio::Morphology load(const std::string& morph_name, unsigned int options) const override { + morphio::Morphology load(const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const override { const auto& derived = static_cast(*this); - return derived.template load_impl(morph_name, options); + return derived.template load_impl(morph_name, options, warning_handler); } - morphio::mut::Morphology load_mut(const std::string& morph_name, - unsigned int options) const override { + morphio::mut::Morphology load_mut( + const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const override { const auto& derived = static_cast(*this); - return derived.template load_impl(morph_name, options); + return derived.template load_impl(morph_name, options, warning_handler); } - std::shared_ptr load_unordered(Collection collection, - std::vector morphology_names, - unsigned int options) const override { + std::shared_ptr load_unordered( + Collection collection, + std::vector morphology_names, + unsigned int options, + std::shared_ptr warning_handler) const override { auto loop_indices = argsort(morphology_names); return std::make_shared(std::move(collection), std::move(loop_indices), std::move(morphology_names), - options); + options, + warning_handler); } }; } // namespace detail @@ -131,8 +147,10 @@ class DirectoryCollection: public morphio::detail::CollectionImpl; template - M load_impl(const std::string& morph_name, unsigned int options) const { - return M(morphology_path(morph_name), options); + M load_impl(const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const { + return M(morphology_path(morph_name), options, warning_handler); } std::string morphology_path(const std::string& morph_name) const { @@ -214,9 +232,11 @@ class HDF5ContainerCollection: public morphio::detail::CollectionImpl; template - M load_impl(const std::string& morph_name, unsigned int options) const { + M load_impl(const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const { std::lock_guard lock(morphio::readers::h5::global_hdf5_mutex()); - return M(_file.getGroup(morph_name), options); + return M(_file.getGroup(morph_name), options, warning_handler); } protected: @@ -261,20 +281,24 @@ Collection::Collection(std::string collection_path, std::vector ext template -typename enable_if_immutable::type Collection::load(const std::string& morph_name, - unsigned int options) const { +typename enable_if_immutable::type Collection::load( + const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const { if (_collection != nullptr) { - return _collection->load(morph_name, options); + return _collection->load(morph_name, options, warning_handler); } throw std::runtime_error("The collection has been closed."); } template -typename enable_if_mutable::type Collection::load(const std::string& morph_name, - unsigned int options) const { +typename enable_if_mutable::type Collection::load( + const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler) const { if (_collection != nullptr) { - return _collection->load_mut(morph_name, options); + return _collection->load_mut(morph_name, options, warning_handler); } throw std::runtime_error("The collection has been closed."); @@ -288,24 +312,34 @@ std::vector Collection::argsort(const std::vector& morpholo throw std::runtime_error("The collection has been closed."); } -template mut::Morphology Collection::load(const std::string& morph_name, - unsigned int options) const; +template mut::Morphology Collection::load( + const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler = nullptr) const; -template Morphology Collection::load(const std::string& morph_name, - unsigned int options) const; +template Morphology Collection::load( + const std::string& morph_name, + unsigned int options, + std::shared_ptr warning_handler = nullptr) const; template LoadUnordered Collection::load_unordered(std::vector morphology_names, - unsigned int options) const { - return LoadUnordered(_collection->load_unordered(*this, morphology_names, options)); + unsigned int options, + std::shared_ptr warning_handler) const { + return LoadUnordered( + _collection->load_unordered(*this, morphology_names, options, warning_handler)); } template LoadUnordered Collection::load_unordered( - std::vector morphology_names, unsigned int options) const; + std::vector morphology_names, + unsigned int options, + std::shared_ptr warning_handler) const; template LoadUnordered Collection::load_unordered( - std::vector morphology_names, unsigned int options) const; + std::vector morphology_names, + unsigned int options, + std::shared_ptr warning_handler) const; void Collection::close() { diff --git a/src/errorMessages.cpp b/src/errorMessages.cpp index f1ebc3cff..435524a27 100644 --- a/src/errorMessages.cpp +++ b/src/errorMessages.cpp @@ -17,19 +17,19 @@ namespace morphio { * -1 will print them all */ void set_maximum_warnings(int32_t n_warnings) { - auto static_handler = getErrorHandler(); + auto static_handler = getWarningHandler(); static_handler->setMaxWarningCount(n_warnings); } /* Whether to raise warning as errors */ void set_raise_warnings(bool is_raise) { - auto static_handler = getErrorHandler(); + auto static_handler = getWarningHandler(); static_handler->setRaiseWarnings(is_raise); } /** Ignore/Unignore a specific warning message */ void set_ignored_warning(enums::Warning warning, bool ignore) { - auto static_handler = getErrorHandler(); + auto static_handler = getWarningHandler(); static_handler->setIgnoredWarning(warning, ignore); } @@ -40,9 +40,9 @@ void set_ignored_warning(const std::vector& warnings, bool ignor } } -std::shared_ptr getErrorHandler() { - static morphio::WarningHandlerPrinter error_handler; - return {std::shared_ptr{}, &error_handler}; +std::shared_ptr getWarningHandler() { + static morphio::WarningHandlerPrinter warning_handler; + return {std::shared_ptr{}, &warning_handler}; } } // namespace morphio diff --git a/src/error_message_generation.cpp b/src/error_message_generation.cpp index 47dbbdfe3..bd33d77d4 100644 --- a/src/error_message_generation.cpp +++ b/src/error_message_generation.cpp @@ -6,11 +6,6 @@ namespace morphio { namespace details { using morphio::readers::ErrorLevel; -bool ErrorMessages::isIgnored(const Warning& warning) { - auto static_handler = getErrorHandler(); - return static_handler->isIgnored(warning); -} - namespace { std::string errorMsg(const std::string& uri, long unsigned int lineNumber, @@ -22,10 +17,6 @@ std::string errorMsg(const std::string& uri, // LCOV_EXCL_START { all the error messages are excluded from coverage -//////////////////////////////////////////////////////////////////////////////// -// ERRORS -//////////////////////////////////////////////////////////////////////////////// - std::string ErrorMessages::ERROR_LINE_NON_PARSABLE(long unsigned int lineNumber) const { return errorMsg(_uri, lineNumber, ErrorLevel::ERROR, "Unable to parse this line"); } diff --git a/src/morphology.cpp b/src/morphology.cpp index 047967842..0b9626b55 100644 --- a/src/morphology.cpp +++ b/src/morphology.cpp @@ -74,7 +74,7 @@ morphio::Property::Properties loadFile(const std::string& path, } if (warning_handler == nullptr) { - warning_handler = morphio::getErrorHandler(); + warning_handler = morphio::getWarningHandler(); } std::string extension = tolower(path.substr(pos + 1)); @@ -101,7 +101,7 @@ morphio::Property::Properties loadString(const std::string& contents, std::string lower_extension = tolower(extension); if (warning_handler == nullptr) { - warning_handler = morphio::getErrorHandler(); + warning_handler = morphio::getWarningHandler(); } if (lower_extension == "asc") { @@ -137,8 +137,10 @@ Morphology::Morphology(const std::string& path, std::shared_ptr warning_handler) : Morphology(loadFile(path, warning_handler, options), options) {} -Morphology::Morphology(const HighFive::Group& group, unsigned int options) - : Morphology(readers::h5::load(group), options) {} +Morphology::Morphology(const HighFive::Group& group, + unsigned int options, + std::shared_ptr warning_handler) + : Morphology(readers::h5::load(group, warning_handler.get()), options) {} Morphology::Morphology(const mut::Morphology& morphology) { properties_ = std::make_shared(morphology.buildReadOnly()); diff --git a/src/mut/morphology.cpp b/src/mut/morphology.cpp index 0eeb40485..7b60b5304 100644 --- a/src/mut/morphology.cpp +++ b/src/mut/morphology.cpp @@ -52,10 +52,12 @@ namespace mut { Morphology::Morphology(const std::string& uri, unsigned int options, std::shared_ptr warning_handler) - : Morphology(morphio::Morphology(uri, options, std::move(warning_handler))) {} + : Morphology(morphio::Morphology(uri, options, warning_handler)) {} -Morphology::Morphology(const HighFive::Group& group, unsigned int options) - : Morphology(morphio::Morphology(group, options)) {} +Morphology::Morphology(const HighFive::Group& group, + unsigned int options, + std::shared_ptr warning_handler) + : Morphology(morphio::Morphology(group, options, warning_handler)) {} Morphology::Morphology(const morphio::mut::Morphology& morphology, unsigned int options, @@ -64,7 +66,7 @@ Morphology::Morphology(const morphio::mut::Morphology& morphology, , _cellProperties(std::make_shared(*morphology._cellProperties)) , _endoplasmicReticulum(morphology.endoplasmicReticulum()) , _dendriticSpineLevel(morphology._dendriticSpineLevel) - , _handler(warning_handler ? warning_handler : getErrorHandler()) { + , _handler(warning_handler ? warning_handler : morphio::getWarningHandler()) { for (const std::shared_ptr
& root : morphology.rootSections()) { appendRootSection(root, true); } @@ -84,7 +86,7 @@ Morphology::Morphology(const morphio::Morphology& morphology, std::make_shared(morphology.properties_->_cellLevel)) , _endoplasmicReticulum(morphology.endoplasmicReticulum()) , _dendriticSpineLevel(morphology.properties_->_dendriticSpineLevel) - , _handler(warning_handler ? warning_handler : getErrorHandler()) { + , _handler(warning_handler != nullptr ? warning_handler : morphio::getWarningHandler()) { for (const morphio::Section& root : morphology.rootSections()) { appendRootSection(root, true); } @@ -131,7 +133,7 @@ std::shared_ptr
Morphology::appendRootSection(const morphio::Section& s const bool emptySection = ptr->points().empty(); if (emptySection) { - getHandler()->emit(std::make_shared(_uri, ptr->id())); + getWarningHandler()->emit(std::make_shared(_uri, ptr->id())); } if (recursive) { @@ -150,7 +152,8 @@ std::shared_ptr
Morphology::appendRootSection(const std::shared_ptrpoints().empty(); if (emptySection) { - getHandler()->emit(std::make_shared(_uri, section_copy->id())); + getWarningHandler()->emit( + std::make_shared(_uri, section_copy->id())); } if (recursive) { @@ -170,7 +173,7 @@ std::shared_ptr
Morphology::appendRootSection(const Property::PointLeve bool emptySection = ptr->points().empty(); if (emptySection) { - getHandler()->emit(std::make_shared(_uri, ptr->id())); + getWarningHandler()->emit(std::make_shared(_uri, ptr->id())); } return ptr; @@ -266,7 +269,7 @@ void Morphology::removeUnifurcations() { bool duplicate = _checkDuplicatePoint(parent, section_); if (!duplicate) { - getHandler()->emit(std::make_shared(_uri, section_, parent)); + getWarningHandler()->emit(std::make_shared(_uri, section_, parent)); } bool isUnifurcation = parent->children().size() == 1; @@ -274,7 +277,7 @@ void Morphology::removeUnifurcations() { // This "if" condition ensures that "unifurcations" (ie. successive // sections with only 1 child) get merged together into a bigger section if (isUnifurcation) { - getHandler()->emit(std::make_shared(_uri, parent->id(), sectionId)); + getWarningHandler()->emit(std::make_shared(_uri, parent->id(), sectionId)); addAnnotation(Property::Annotation(AnnotationType::SINGLE_CHILD, sectionId, diff --git a/src/mut/section.cpp b/src/mut/section.cpp index 16911cb64..83d2738d6 100644 --- a/src/mut/section.cpp +++ b/src/mut/section.cpp @@ -223,7 +223,7 @@ std::shared_ptr
Section::appendSection(const Property::PointLevel& poin } void Section::emitWarning(std::shared_ptr wm) { - getOwningMorphologyOrThrow()->getHandler()->emit(std::move(wm)); + getOwningMorphologyOrThrow()->getWarningHandler()->emit(std::move(wm)); } } // end namespace mut diff --git a/src/readers/morphologyHDF5.cpp b/src/readers/morphologyHDF5.cpp index b33d92409..fc6a3725f 100644 --- a/src/readers/morphologyHDF5.cpp +++ b/src/readers/morphologyHDF5.cpp @@ -81,9 +81,12 @@ Property::Properties load(const std::string& uri, WarningHandler* warning_handle } } -Property::Properties load(const HighFive::Group& group) { +Property::Properties load(const HighFive::Group& group, WarningHandler* warning_handler) { std::lock_guard lock(morphio::readers::h5::global_hdf5_mutex()); - return MorphologyHDF5(group).load(getErrorHandler().get()); + if (warning_handler == nullptr) { + warning_handler = getWarningHandler().get(); + } + return MorphologyHDF5(group).load(warning_handler); } Property::Properties MorphologyHDF5::load(WarningHandler* warning_handler) { diff --git a/src/readers/morphologyHDF5.h b/src/readers/morphologyHDF5.h index 161b4e9bd..0a63bc122 100644 --- a/src/readers/morphologyHDF5.h +++ b/src/readers/morphologyHDF5.h @@ -16,7 +16,7 @@ namespace morphio { namespace readers { namespace h5 { Property::Properties load(const std::string& uri, WarningHandler*); -Property::Properties load(const HighFive::Group& group); +Property::Properties load(const HighFive::Group& group, WarningHandler*); class MorphologyHDF5 { diff --git a/src/readers/morphologySWC.cpp b/src/readers/morphologySWC.cpp index 287b12e4b..f456a6af4 100644 --- a/src/readers/morphologySWC.cpp +++ b/src/readers/morphologySWC.cpp @@ -285,7 +285,7 @@ class SWCBuilder return ret; } - SomaType somaType(WarningHandler* h) { + SomaType somaType(mut::Morphology& morph, WarningHandler* h) { switch (morph.soma()->points().size()) { case 0: { return SOMA_UNDEFINED; @@ -338,7 +338,7 @@ class SWCBuilder Property::Properties buildProperties(const std::string& contents, unsigned int options, std::shared_ptr& h) { - morph.setErrorHandler(h); + mut::Morphology morph{h}; readSamples(contents); for (const auto& sample_pair : samples) { @@ -366,7 +366,7 @@ class SWCBuilder } if (isSectionStart(sample)) { - _processSectionStart(sample); + _processSectionStart(morph, sample); } else if (sample.type != SECTION_SOMA) { swcIdToSectionId[sample.id] = swcIdToSectionId[static_cast(sample.parentId)]; @@ -386,7 +386,7 @@ class SWCBuilder morph.applyModifiers(options); Property::Properties properties = morph.buildReadOnly(); - properties._cellLevel._somaType = somaType(h.get()); + properties._cellLevel._somaType = somaType(morph, h.get()); h->setIgnoredWarning(morphio::Warning::APPENDING_EMPTY_SECTION, originalIsIgnored); @@ -397,7 +397,7 @@ class SWCBuilder append last point of previous section if current section is not a root section and update the parent ID of the new section */ - void _processSectionStart(const SWCSample& sample) { + void _processSectionStart(mut::Morphology& morph, const SWCSample& sample) { Property::PointLevel properties; uint32_t id = 0; @@ -436,7 +436,6 @@ class SWCBuilder unsigned int lastSomaPoint = 0; std::unordered_map> children; std::unordered_map samples; - mut::Morphology morph; std::string uri; }; diff --git a/tests/test_11_collection.py b/tests/test_11_collection.py index 93fae36f1..100c90ee5 100644 --- a/tests/test_11_collection.py +++ b/tests/test_11_collection.py @@ -38,7 +38,6 @@ def check_load_from_collection(collection): morph = collection.load(morph_name, mutable=True) assert isinstance(morph, morphio.mut.Morphology) - @pytest.mark.parametrize("collection_path", COLLECTION_PATHS) def test_load_from_collection_with_context(collection_path): with morphio.Collection(collection_path) as collection: @@ -81,3 +80,9 @@ def test_container_unordered1(collection_path): sorted(loop_indices), np.arange(len(morphology_names)) ) + +def test_container_with_warning_handler(): + with morphio.Collection(DATA_DIR) as collection: + warning_handler = morphio.WarningHandlerCollector() + collection.load('neurite_wrong_root_point', warning_handler=warning_handler) + assert len(warning_handler.get_all()) == 3 diff --git a/tests/test_1_swc.py b/tests/test_1_swc.py index 70cd114c9..db14aee4e 100644 --- a/tests/test_1_swc.py +++ b/tests/test_1_swc.py @@ -521,7 +521,7 @@ def test_no_soma(): def test_WarningHandlerCollector(): warnings = morphio.WarningHandlerCollector() - Morphology(str(DATA_DIR / 'neurite_wrong_root_point.swc'), warning_handler=warnings) + Morphology(DATA_DIR / 'neurite_wrong_root_point.swc', warning_handler=warnings) assert len(warnings.get_all()) == 3 assert [True, True, False] == [e.was_marked_ignore for e in warnings.get_all()] assert warnings.get_all()[2].warning.line_numbers[0] == 4 diff --git a/tests/test_4_immut.py b/tests/test_4_immut.py index 1564f0c40..3c38f0f5e 100644 --- a/tests/test_4_immut.py +++ b/tests/test_4_immut.py @@ -1,6 +1,5 @@ # Copyright (c) 2013-2023, EPFL/Blue Brain Project # SPDX-License-Identifier: Apache-2.0 -from collections import OrderedDict from pathlib import Path import numpy as np @@ -15,11 +14,11 @@ DATA_DIR = Path(__file__).parent / "data" # These 3 cells are identical -CELLS = OrderedDict({ +CELLS = { 'asc': Morphology(DATA_DIR / "simple.asc"), 'swc': Morphology(DATA_DIR / "simple.swc"), 'h5': Morphology(DATA_DIR / "h5/v1/simple.h5"), -}) +} def test_heterogeneous_sections(): diff --git a/tests/test_immutable_morphology.cpp b/tests/test_immutable_morphology.cpp index 3738dc88e..ce3e2bc7f 100644 --- a/tests/test_immutable_morphology.cpp +++ b/tests/test_immutable_morphology.cpp @@ -106,11 +106,11 @@ TEST_CASE("heterogeneous-sections", "[immutableMorphology]") { } } -TEST_CASE("modifers", "[immutableMorphology]") { +TEST_CASE("modifiers", "[immutableMorphology]") { morphio::Morphology morphNoModifier = morphio::Morphology( "data/reversed_NRN_neurite_order.swc"); std::vector rootSectionTypesNoModifier; - for (auto sectionNoMod : morphNoModifier.rootSections()) { + for (const auto& sectionNoMod : morphNoModifier.rootSections()) { rootSectionTypesNoModifier.push_back(sectionNoMod.type()); } REQUIRE(rootSectionTypesNoModifier == std::vector{ @@ -123,7 +123,7 @@ TEST_CASE("modifers", "[immutableMorphology]") { morphio::Option::NRN_ORDER); std::vector rootSectionTypes; - for (auto section : morph.rootSections()) { + for (const auto& section : morph.rootSections()) { rootSectionTypes.push_back(section.type()); } REQUIRE(rootSectionTypes.size() == 3); @@ -137,7 +137,7 @@ TEST_CASE("modifers", "[immutableMorphology]") { morphio::Option::NRN_ORDER); std::vector rootSectionTypesH5; - for (auto section : morphModifierh5.rootSections()) { + for (const auto& section : morphModifierh5.rootSections()) { rootSectionTypesH5.push_back(section.type()); } // Should be inverted without the option