From 92e6960594a5780e9c454f2161800d532688df41 Mon Sep 17 00:00:00 2001 From: MikeG Date: Tue, 4 Jul 2023 12:40:41 +0200 Subject: [PATCH] remove excessive static_casts in SWC reader (#466) * don't allow negative IDs, except for the special parent == -1, for the first sample --- include/morphio/errorMessages.h | 5 +- src/readers/morphologySWC.cpp | 113 ++++++++++++++++++-------------- 2 files changed, 65 insertions(+), 53 deletions(-) diff --git a/include/morphio/errorMessages.h b/include/morphio/errorMessages.h index 109045eb4..103a27c1c 100644 --- a/include/morphio/errorMessages.h +++ b/include/morphio/errorMessages.h @@ -66,14 +66,15 @@ static std::set _ignoredWarnings; /** A sample of section for error reporting, includes its position (line) within the file. **/ struct Sample { + enum : unsigned int { UNKNOWN_ID = 0xFFFFFFFE }; Sample() = default; floatType diameter = -1.; bool valid = false; Point point{}; SectionType type = SECTION_UNDEFINED; - int parentId = -1; - unsigned int id = 0; + unsigned int parentId = UNKNOWN_ID; + unsigned int id = UNKNOWN_ID; unsigned int lineNumber = 0; }; diff --git a/src/readers/morphologySWC.cpp b/src/readers/morphologySWC.cpp index d0614219e..8b9744c00 100644 --- a/src/readers/morphologySWC.cpp +++ b/src/readers/morphologySWC.cpp @@ -1,5 +1,6 @@ #include "morphologySWC.h" +#include #include // uint32_t #include // std::shared_ptr #include // std::stringstream @@ -14,6 +15,10 @@ #include namespace { +// It's not clear if -1 is the only way of identifying the root section. +const int SWC_UNDEFINED_PARENT = -1; +const unsigned int SWC_ROOT = 0xFFFFFFFD; + bool _ignoreLine(const std::string& line) { std::size_t pos = line.find_first_not_of("\n\r\t "); return pos == std::string::npos || line[pos] == '#'; @@ -21,23 +26,38 @@ bool _ignoreLine(const std::string& line) { morphio::readers::Sample readSample(const char* line, unsigned int lineNumber_) { #ifdef MORPHIO_USE_DOUBLE - const char* const format = "%u%d%lg%lg%lg%lg%d"; + const char* const format = "%d%d%lg%lg%lg%lg%d"; #else - const char* const format = "%u%d%f%f%f%f%d"; + const char* const format = "%d%d%f%f%f%f%d"; #endif morphio::floatType radius = -1.; int int_type = -1; morphio::readers::Sample sample; + int parentId = -1; + int id = -1; sample.valid = sscanf(line, format, - &sample.id, + &id, &int_type, &sample.point[0], &sample.point[1], &sample.point[2], &radius, - &sample.parentId) == 7; + &parentId) == 7; + + if (id < 0) { + throw morphio::RawDataError("Negative IDs are not supported"); + } + sample.id = static_cast(id); + + if (parentId < -1) { + throw morphio::RawDataError("Negative ParentIDs are not supported"); + } else if (parentId == SWC_UNDEFINED_PARENT) { + sample.parentId = SWC_ROOT; + } else { + sample.parentId = static_cast(parentId); + } sample.type = static_cast(int_type); sample.diameter = radius * 2; // The point array stores diameters. @@ -50,9 +70,6 @@ morphio::readers::Sample readSample(const char* line, unsigned int lineNumber_) namespace morphio { namespace readers { namespace swc { -// It's not clear if -1 is the only way of identifying a root section. -const int SWC_UNDEFINED_PARENT = -1; - /** Parsing SWC according to this specification: http://www.neuronland.org/NLMorphologyConverter/MorphologyFormats/SWC/Spec.html @@ -93,18 +110,18 @@ class SWCBuilder children[sample.parentId].push_back(sample.id); if (sample.type == SECTION_SOMA) { - lastSomaPoint = static_cast(sample.id); + lastSomaPoint = sample.id; } } } /** Are considered potential somata all sample - with parentId == -1 and sample.type == SECTION_SOMA + with parentId == SWC_ROOT and sample.type == SECTION_SOMA **/ std::vector _potentialSomata() { std::vector somata; - for (auto id : children[SWC_UNDEFINED_PARENT]) { + for (auto id : children[SWC_ROOT]) { if (samples[id].type == SECTION_SOMA) { somata.push_back(samples[id]); } @@ -117,10 +134,9 @@ class SWCBuilder return; } - if (sample.parentId != SWC_UNDEFINED_PARENT && - !children[static_cast(sample.id)].empty()) { + if (sample.parentId != SWC_ROOT && !children[sample.id].empty()) { std::vector soma_bifurcations; - for (auto id : children[static_cast(sample.id)]) { + for (unsigned int id : children[sample.id]) { if (samples[id].type == SECTION_SOMA) { soma_bifurcations.push_back(samples[id]); } else { @@ -133,21 +149,20 @@ class SWCBuilder } } - if (sample.parentId != -1 && - samples.count(static_cast(sample.parentId)) > 0 && - samples[static_cast(sample.parentId)].type != SECTION_SOMA) { + if (sample.parentId != SWC_ROOT && samples.count(sample.parentId) > 0 && + samples[sample.parentId].type != SECTION_SOMA) { throw morphio::SomaError(err.ERROR_SOMA_WITH_NEURITE_PARENT(sample)); } } void raiseIfSelfParent(const Sample& sample) { - if (sample.parentId == static_cast(sample.id)) { + if (sample.parentId == sample.id) { throw morphio::RawDataError(err.ERROR_SELF_PARENT(sample)); } } void warnIfDisconnectedNeurite(const Sample& sample) { - if (sample.parentId == SWC_UNDEFINED_PARENT && sample.type != SECTION_SOMA) { + if (sample.parentId == SWC_ROOT && sample.type != SECTION_SOMA) { printError(Warning::DISCONNECTED_NEURITE, err.WARNING_DISCONNECTED_NEURITE(sample)); } } @@ -170,8 +185,7 @@ class SWCBuilder } void raiseIfNoParent(const Sample& sample) { - if (sample.parentId > -1 && - samples.count(static_cast(sample.parentId)) == 0) { + if (sample.parentId != SWC_ROOT && samples.count(sample.parentId) == 0) { throw morphio::MissingParentError(err.ERROR_MISSING_PARENT(sample)); } } @@ -182,33 +196,23 @@ class SWCBuilder } } - /** - A neurite which is not attached to the soma - **/ - bool isOrphanNeurite(const Sample& sample) { - return (sample.parentId == SWC_UNDEFINED_PARENT && sample.type != SECTION_SOMA); - } - bool isRootPoint(const Sample& sample) { - return isOrphanNeurite(sample) || + bool isOrphanNeurite = sample.parentId == SWC_ROOT && sample.type != SECTION_SOMA; + return isOrphanNeurite || (sample.type != SECTION_SOMA && - samples[static_cast(sample.parentId)].type == - SECTION_SOMA); // Exclude soma bifurcations + samples[sample.parentId].type == SECTION_SOMA); // Exclude soma bifurcations } bool isSectionStart(const Sample& sample) { return (isRootPoint(sample) || - (sample.parentId > -1 && - isSectionEnd(samples[static_cast(sample.parentId)]))); // Standard - // section + (sample.parentId != SWC_ROOT && + isSectionEnd(samples[sample.parentId]))); // Standard section } bool isSectionEnd(const Sample& sample) { - int id = static_cast(sample.id); - return id == lastSomaPoint || // End of soma - children[id].empty() || // Reached leaf - (children[id].size() >= 2 && // Reached neurite - // bifurcation + return sample.id == lastSomaPoint || // End of soma + children[sample.id].empty() || // Reached leaf + (children[sample.id].size() >= 2 && // Reached neurite bifurcation sample.type != SECTION_SOMA); } @@ -218,11 +222,19 @@ class SWCBuilder somaOrSection->diameters().push_back(sample.diameter); } - void _pushChildren(std::vector& vec, int32_t id) { - for (unsigned int childId : children[id]) { - vec.push_back(childId); - _pushChildren(vec, static_cast(childId)); - } + std::vector constructDepthFirstSamples() { + std::vector ret; + ret.reserve(samples.size()); + const auto pushChildren = [&](const auto& f, unsigned int id) -> void { + for (unsigned int childId : children[id]) { + ret.push_back(childId); + f(f, childId); + } + }; + + pushChildren(pushChildren, SWC_ROOT); + + return ret; } void raiseIfNonConform(const Sample& sample) { @@ -278,11 +290,11 @@ class SWCBuilder // NeuroMorpho format is characterized by a 3 points soma // with a bifurcation at soma root case 3: { - uint32_t somaRootId = children[-1][0]; - auto& somaChildren = children[static_cast(somaRootId)]; + uint32_t somaRootId = children[SWC_ROOT][0]; + const auto& somaChildren = children[somaRootId]; std::vector children_soma_points; - for (auto child : somaChildren) { + for (unsigned int child : somaChildren) { if (this->samples[child].type == SECTION_SOMA) { children_soma_points.push_back(this->samples[child]); } @@ -323,8 +335,7 @@ class SWCBuilder bool originalIsIgnored = err.isIgnored(morphio::Warning::APPENDING_EMPTY_SECTION); set_ignored_warning(morphio::Warning::APPENDING_EMPTY_SECTION, true); - std::vector depthFirstSamples; - _pushChildren(depthFirstSamples, -1); + std::vector depthFirstSamples = constructDepthFirstSamples(); for (const auto id : depthFirstSamples) { const Sample& sample = samples[id]; @@ -402,9 +413,9 @@ class SWCBuilder // 3-pts soma std::vector neurite_wrong_root; - int lastSomaPoint = -1; - std::unordered_map> children; - std::unordered_map samples; + unsigned int lastSomaPoint = 0; + std::unordered_map> children; + std::unordered_map samples; mut::Morphology morph; ErrorMessages err; };