Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

don't use exceptions when non-exceptional methods are available #389

Merged
merged 6 commits into from
Mar 18, 2022
Merged

don't use exceptions when non-exceptional methods are available #389

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
89800b6
don't use exceptions when non-exceptional methods are available
mgeplf Mar 1, 2022
8634bc4
fix catch in section_base.h
mgeplf Mar 4, 2022
f0785ab
fix catch in errorMessages.h
mgeplf Mar 4, 2022
c65fdff
code review feedback, some cleanup
mgeplf Mar 16, 2022
24073d9
Merge remote-tracking branch 'origin/master' into fix-exceptions
mgeplf Mar 16, 2022
34bff1a
write more tests so we increase the test coverage
mgeplf Mar 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions include/morphio/errorMessages.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ struct DebugInfo {

/** Get section's line number within morphology file */
int32_t getLineNumber(uint32_t sectionId) const {
try {
return _lineNumbers.at(sectionId);
} catch (const std::out_of_range&) {
const auto it = _lineNumbers.find(sectionId);
if (it == _lineNumbers.end()) {
return -1;
}
return it->second;
}
/** Morphology filename */
std::string _filename;
Expand Down
4 changes: 2 additions & 2 deletions include/morphio/morphology.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class Morphology
/** Constructor from an already parsed file */
explicit Morphology(const HighFive::Group& group, unsigned int options = NO_MODIFIER);
/** Constructor from an instance of morphio::mut::Morphology */
explicit Morphology(mut::Morphology);
explicit Morphology(const mut::Morphology&);

/** Return the soma object */
Soma soma() const;
Expand All @@ -52,7 +52,7 @@ class Morphology
Mitochondria mitochondria() const;

/** Return the endoplasmic reticulum object */
const EndoplasmicReticulum endoplasmicReticulum() const;
EndoplasmicReticulum endoplasmicReticulum() const;

/** Return the annotation object */
const std::vector<Property::Annotation>& annotations() const;
Expand Down
2 changes: 1 addition & 1 deletion include/morphio/mut/mitochondria.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class Mitochondria
/**
Append a new root MitoSection
**/
MitoSectionP appendRootSection(const Property::MitochondriaPointLevel& points);
MitoSectionP appendRootSection(const Property::MitochondriaPointLevel& pointProperties);

/**
Append a root MitoSection
Expand Down
28 changes: 17 additions & 11 deletions include/morphio/section_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,25 @@ T SectionBase<T>::parent() const {

template <typename T>
std::vector<T> SectionBase<T>::children() const {
const auto& section_children = properties_->children<typename T::SectionId>();

if (section_children.empty()) {
return {};
}

const auto it = section_children.find(static_cast<int>(id_));
if (it == section_children.end()) {
return {};
}

std::vector<T> result;
try {
const std::vector<uint32_t>& _children = properties_->children<typename T::SectionId>().at(
static_cast<int>(id_));
result.reserve(_children.size());
for (uint32_t id : _children) {
result.push_back(T(id, properties_));
}

return result;
} catch (const std::out_of_range&) {
return result;
const std::vector<uint32_t> children = it->second;
result.reserve(children.size());
for (uint32_t id : children) {
result.push_back(T(id, properties_));
}

return result;
}


Expand Down
157 changes: 79 additions & 78 deletions src/morphology.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,72 @@
#include "readers/morphologyHDF5.h"
#include "readers/morphologySWC.h"

namespace {

void buildChildren(const std::shared_ptr<morphio::Property::Properties> properties) {
{
const auto& sections = properties->get<morphio::Property::Section>();
auto& children = properties->_sectionLevel._children;

for (unsigned int i = 0; i < sections.size(); ++i) {
const int32_t parent = sections[i][1];
children[parent].push_back(i);
}
}

{
const auto& sections = properties->get<morphio::Property::MitoSection>();
auto& children = properties->_mitochondriaSectionLevel._children;

for (unsigned int i = 0; i < sections.size(); ++i) {
const int32_t parent = sections[i][1];
children[parent].push_back(i);
}
}
}

morphio::SomaType getSomaType(long unsigned int num_soma_points) {
switch (num_soma_points) {
case 0:
return morphio::SOMA_UNDEFINED;
case 1:
return morphio::SOMA_SINGLE_POINT;
case 2:
return morphio::SOMA_UNDEFINED;
default:;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is how clang-format wanted this; FYI

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could fold the return below into the switch, but maybe some other linting thing then complains?

}
return morphio::SOMA_SIMPLE_CONTOUR;
}

morphio::Property::Properties loadURI(const std::string& source, unsigned int options) {
const size_t pos = source.find_last_of('.');
if (pos == std::string::npos) {
throw(morphio::UnknownFileType("File has no extension"));
}

// Cross-platform check of file existance
std::ifstream file(source.c_str());
if (!file) {
throw(morphio::RawDataError("File: " + source + " does not exist."));
}

std::string extension = source.substr(pos);

if (extension == ".h5" || extension == ".H5") {
return morphio::readers::h5::load(source);
} else if (extension == ".asc" || extension == ".ASC") {
return morphio::readers::asc::load(source, options);
} else if (extension == ".swc" || extension == ".SWC") {
return morphio::readers::swc::load(source, options);
}

throw(morphio::UnknownFileType("Unhandled file type: only SWC, ASC and H5 are supported"));
}


} // namespace

namespace morphio {
void buildChildren(std::shared_ptr<Property::Properties> properties);
SomaType getSomaType(long unsigned int num_soma_point);
Property::Properties loadURI(const std::string& source, unsigned int options);

Morphology::Morphology(const Property::Properties& properties, unsigned int options)
: properties_(std::make_shared<Property::Properties>(properties)) {
Expand Down Expand Up @@ -44,7 +106,7 @@ Morphology::Morphology(const HighFive::Group& group, unsigned int options)
Morphology::Morphology(const std::string& source, unsigned int options)
: Morphology(loadURI(source, options), options) {}

Morphology::Morphology(mut::Morphology morphology) {
Morphology::Morphology(const mut::Morphology& morphology) {
properties_ = std::make_shared<Property::Properties>(morphology.buildReadOnly());
buildChildren(properties_);
}
Expand All @@ -57,7 +119,7 @@ Mitochondria Morphology::mitochondria() const {
return Mitochondria(properties_);
}

const EndoplasmicReticulum Morphology::endoplasmicReticulum() const {
EndoplasmicReticulum Morphology::endoplasmicReticulum() const {
return EndoplasmicReticulum(properties_);
}

Expand All @@ -74,19 +136,21 @@ Section Morphology::section(uint32_t id) const {
}

std::vector<Section> Morphology::rootSections() const {
const auto& sections = properties_->children<morphio::Property::Section>();

if (sections.empty()) {
return {};
}

std::vector<Section> result;
try {
const std::vector<uint32_t>& children =
properties_->children<morphio::Property::Section>().at(-1);
result.reserve(children.size());
for (auto id : children) {
result.push_back(section(id));
}

return result;
} catch (const std::out_of_range&) {
return result;
const std::vector<uint32_t>& children = sections.at(-1);
result.reserve(children.size());
for (auto id : children) {
result.push_back(section(id));
}

return result;
}

std::vector<Section> Morphology::sections() const {
Expand Down Expand Up @@ -165,67 +229,4 @@ breadth_iterator Morphology::breadth_end() const {
return breadth_iterator();
}

SomaType getSomaType(long unsigned int num_soma_points) {
try {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to anonymous namespace and replaced the try w/ a switch

return std::map<long unsigned int, SomaType>{{0, SOMA_UNDEFINED},
{1, SOMA_SINGLE_POINT},
{2, SOMA_UNDEFINED}}
.at(num_soma_points);
} catch (const std::out_of_range&) {
return SOMA_SIMPLE_CONTOUR;
}
}

void buildChildren(std::shared_ptr<Property::Properties> properties) {
{
const auto& sections = properties->get<Property::Section>();
auto& children = properties->_sectionLevel._children;

for (unsigned int i = 0; i < sections.size(); ++i) {
const int32_t parent = sections[i][1];
children[parent].push_back(i);
}
}

{
const auto& sections = properties->get<Property::MitoSection>();
auto& children = properties->_mitochondriaSectionLevel._children;

for (unsigned int i = 0; i < sections.size(); ++i) {
const int32_t parent = sections[i][1];
children[parent].push_back(i);
}
}
}

Property::Properties loadURI(const std::string& source, unsigned int options) {
const size_t pos = source.find_last_of(".");
if (pos == std::string::npos) {
throw(UnknownFileType("File has no extension"));
}

// Cross-platform check of file existance
std::ifstream file(source.c_str());
if (!file) {
throw(RawDataError("File: " + source + " does not exist."));
}

std::string extension = source.substr(pos);

auto loader = [&source, &options, &extension]() {
if (extension == ".h5" || extension == ".H5") {
return readers::h5::load(source);
}
if (extension == ".asc" || extension == ".ASC") {
return readers::asc::load(source, options);
}
if (extension == ".swc" || extension == ".SWC") {
return readers::swc::load(source, options);
}
throw(UnknownFileType("Unhandled file type: only SWC, ASC and H5 are supported"));
};

return loader();
}

} // namespace morphio
7 changes: 1 addition & 6 deletions src/mut/mito_section.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,7 @@ std::shared_ptr<MitoSection> MitoSection::parent() const {
}

bool MitoSection::isRoot() const {
try {
parent();
return false;
} catch (const std::out_of_range&) {
return true;
}
return mitochondria_->parent_.count(id()) == 0;
}

bool MitoSection::hasSameShape(const MitoSection& other) const noexcept {
Expand Down
22 changes: 8 additions & 14 deletions src/mut/mitochondria.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace mut {

Mitochondria::MitoSectionP Mitochondria::appendRootSection(const morphio::MitoSection& section_,
bool recursive) {
const auto ptr = std::make_shared<MitoSection>(this, _counter, section_);
auto ptr = std::make_shared<MitoSection>(this, _counter, section_);
_register(ptr);
root_sections_.push_back(ptr);

Expand All @@ -25,7 +25,7 @@ Mitochondria::MitoSectionP Mitochondria::appendRootSection(const morphio::MitoSe

Mitochondria::MitoSectionP Mitochondria::appendRootSection(const MitoSectionP& section_,
bool recursive) {
const auto section_copy = std::make_shared<MitoSection>(this, _counter, *section_);
auto section_copy = std::make_shared<MitoSection>(this, _counter, *section_);
_register(section_copy);
root_sections_.push_back(section_copy);

Expand All @@ -40,7 +40,7 @@ Mitochondria::MitoSectionP Mitochondria::appendRootSection(const MitoSectionP& s

Mitochondria::MitoSectionP Mitochondria::appendRootSection(
const Property::MitochondriaPointLevel& pointProperties) {
const auto ptr = std::make_shared<MitoSection>(this, _counter, pointProperties);
auto ptr = std::make_shared<MitoSection>(this, _counter, pointProperties);
_register(ptr);
root_sections_.push_back(ptr);

Expand Down Expand Up @@ -70,12 +70,7 @@ const Mitochondria::MitoSectionP& Mitochondria::parent(const MitoSectionP& paren
}

bool Mitochondria::isRoot(const MitoSectionP& section_) const {
try {
parent(section_);
return false;
} catch (const std::out_of_range&) {
return true;
}
return parent_.count(section_->id()) == 0;
}

const Mitochondria::MitoSectionP& Mitochondria::section(uint32_t id) const {
Expand All @@ -86,14 +81,13 @@ void Mitochondria::_buildMitochondria(Property::Properties& properties) const {
int32_t counter = 0;
std::map<uint32_t, int32_t> newIds;

for (std::shared_ptr<MitoSection> mitoStart : root_sections_) {
for (const std::shared_ptr<MitoSection>& mitoStart : root_sections_) {
std::queue<std::shared_ptr<MitoSection>> q;
q.push(mitoStart);
while (!q.empty()) {
std::shared_ptr<MitoSection> section_ = q.front();
q.pop();
bool root = isRoot(section_);
int32_t parentOnDisk = root ? -1 : newIds[parent(section_)->id()];
int32_t parentOnDisk = isRoot(section_) ? -1 : newIds[parent(section_)->id()];

properties._mitochondriaSectionLevel._sections.push_back(
{static_cast<int>(properties._mitochondriaPointLevel._diameters.size()),
Expand All @@ -102,7 +96,7 @@ void Mitochondria::_buildMitochondria(Property::Properties& properties) const {

newIds[section_->id()] = counter++;

for (auto child : children(section_)) {
for (const auto& child : children(section_)) {
q.push(child);
}
}
Expand Down Expand Up @@ -138,7 +132,7 @@ mito_upstream_iterator Mitochondria::upstream_end() const {
}

uint32_t Mitochondria::_register(const MitoSectionP& section) {
if (sections_.count(section->id())) {
if (sections_.count(section->id()) > 0) {
throw SectionBuilderError("Section already exists");
}
_counter = std::max(_counter, section->id()) + 1;
Expand Down
2 changes: 1 addition & 1 deletion tests/test_3_h5.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def test_soma_no_neurite():
[ 1., 0., 0.],
[ 0., 25., 0.]])
assert_array_equal(n.soma.diameters,
[6,6,15])
[6, 6, 15])

assert len(n.root_sections) == 0

Expand Down
4 changes: 2 additions & 2 deletions tests/test_morphology_readers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ TEST_CASE("LoadH5Glia", "[morphology]") {
{
const morphio::Morphology m("data/h5/v1/glia_soma_only.h5");
REQUIRE(m.soma().points().size() == 1);
REQUIRE(m.points().size() == 0);
REQUIRE(m.perimeters().size() == 0);
REQUIRE(m.points().empty());
REQUIRE(m.perimeters().empty());
}

{ // empty perimeters
Expand Down