Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in UpdateFromDict #231

Merged
merged 2 commits into from
Jan 11, 2024
Merged
Changes from all commits
Commits
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
115 changes: 58 additions & 57 deletions pycolmap/helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "pycolmap/log_exceptions.h"

#include <exception>
#include <memory>
#include <regex>
#include <sstream>
Expand All @@ -33,17 +34,16 @@ const Eigen::IOFormat vec_fmt(Eigen::StreamPrecision,

template <typename T>
inline T pyStringToEnum(const py::enum_<T>& enm, const std::string& value) {
auto values = enm.attr("__members__").template cast<py::dict>();
auto strVal = py::str(value);
if (values.contains(strVal)) {
return T(values[strVal].template cast<T>());
const auto values = enm.attr("__members__").template cast<py::dict>();
const auto str_val = py::str(value);
if (values.contains(str_val)) {
return T(values[str_val].template cast<T>());
}
std::string msg =
const std::string msg =
"Invalid string value " + value + " for enum " +
std::string(enm.attr("__name__").template cast<std::string>());
THROW_EXCEPTION(std::out_of_range, msg.c_str());
T t;
return t;
return T();
}

template <typename T>
Expand All @@ -54,25 +54,31 @@ inline void AddStringToEnumConstructor(py::enum_<T>& enm) {
py::implicitly_convertible<std::string, T>();
}

inline void UpdateFromDict(py::object& self, py::dict& dict) {
for (auto& it : dict) {
inline void UpdateFromDict(py::object& self, const py::dict& dict) {
for (const auto& it : dict) {
if (!py::isinstance<py::str>(it.first)) {
const std::string msg = "Dictionary key is not a string: " +
py::str(it.first).template cast<std::string>();
THROW_EXCEPTION(std::invalid_argument, msg.c_str());
}
const py::str name = py::reinterpret_borrow<py::str>(it.first);
const py::handle& value = it.second;
const auto attr = self.attr(name);
try {
if (py::hasattr(self.attr(it.first), "mergedict")) {
self.attr(it.first).attr("mergedict").attr("__call__")(it.second);
if (py::hasattr(attr, "mergedict") && py::isinstance<py::dict>(value)) {
attr.attr("mergedict").attr("__call__")(value);
} else {
self.attr(it.first) = it.second;
self.attr(name) = value;
}
} catch (const py::error_already_set& ex) {
if (ex.matches(PyExc_TypeError)) {
// If fail we try bases of the class
py::list bases = self.attr(it.first)
.attr("__class__")
.attr("__bases__")
.cast<py::list>();
const py::list bases =
attr.attr("__class__").attr("__bases__").cast<py::list>();
bool success_on_base = false;
for (auto& base : bases) {
try {
self.attr(it.first) = base(it.second);
self.attr(name) = base(value);
success_on_base = true;
break;
} catch (const py::error_already_set&) {
Expand All @@ -86,31 +92,27 @@ inline void UpdateFromDict(py::object& self, py::dict& dict) {
ss << self.attr("__class__")
.attr("__name__")
.template cast<std::string>()
<< "." << py::str(it.first).template cast<std::string>()
<< ": Could not convert "
<< py::type::of(it.second.cast<py::object>())
.attr("__name__")
.template cast<std::string>()
<< ": " << py::str(it.second).template cast<std::string>() << " to '"
<< py::type::of(self.attr(it.first))
<< "." << name.template cast<std::string>() << ": Could not convert "
<< py::type::of(value.cast<py::object>())
.attr("__name__")
.template cast<std::string>()
<< ": " << py::str(value).template cast<std::string>() << " to '"
<< py::type::of(attr).attr("__name__").template cast<std::string>()
<< "'.";
// We write the err message to give info even if exceptions
// is catched outside, e.g. in function overload resolve
LOG(ERROR) << "Internal TypeError: " << ss.str();
throw(py::type_error(std::string("Failed to merge dict into class: ") +
"Could not assign " +
py::str(it.first).template cast<std::string>()));
name.template cast<std::string>()));
} else if (ex.matches(PyExc_AttributeError) &&
py::str(ex.value()).cast<std::string>() ==
std::string("can't set attribute")) {
std::stringstream ss;
ss << self.attr("__class__")
.attr("__name__")
.template cast<std::string>()
<< "." << py::str(it.first).template cast<std::string>()
<< " defined readonly.";
<< "." << name.template cast<std::string>() << " defined readonly.";
throw py::attribute_error(ss.str());
} else if (ex.matches(PyExc_AttributeError)) {
LOG(ERROR) << "Internal AttributeError: "
Expand All @@ -125,33 +127,32 @@ inline void UpdateFromDict(py::object& self, py::dict& dict) {
}
}

bool AttributeIsFunction(const std::string& name, const py::object& attribute) {
return (name.find("__") == 0 || name.rfind("__") != std::string::npos ||
py::hasattr(attribute, "__func__") ||
py::hasattr(attribute, "__call__"));
}

template <typename T, typename... options>
inline py::dict ConvertToDict(const T& self) {
auto pyself = py::cast(self);
const auto pyself = py::cast(self);
py::dict dict;
for (auto& handle : pyself.attr("__dir__")()) {
std::string attribute = py::str(handle);
auto member = pyself.attr(attribute.c_str());
if (attribute.find("__") != 0 &&
attribute.rfind("__") == std::string::npos &&
!py::hasattr(member, "__func__")) {
if (py::hasattr(member, "todict")) {
dict[attribute.c_str()] =
member.attr("todict").attr("__call__")().template cast<py::dict>();
} else {
dict[attribute.c_str()] = member;
}
for (const auto& handle : pyself.attr("__dir__")()) {
const py::str name = py::reinterpret_borrow<py::str>(handle);
const auto attribute = pyself.attr(name);
if (AttributeIsFunction(name, attribute)) {
continue;
}
if (py::hasattr(attribute, "todict")) {
dict[name] =
attribute.attr("todict").attr("__call__")().template cast<py::dict>();
} else {
dict[name] = attribute;
}
}
return dict;
}

bool AttributeIsFunction(const std::string& name, const py::object& attribute) {
return (name.find("__") == 0 || name.rfind("__") != std::string::npos ||
py::hasattr(attribute, "__func__") ||
py::hasattr(attribute, "__call__"));
}

template <typename T, typename... options>
inline std::string CreateSummary(const T& self, bool write_type) {
std::stringstream ss;
Expand All @@ -161,38 +162,38 @@ inline std::string CreateSummary(const T& self, bool write_type) {
ss << pyself.attr("__class__").attr("__name__").template cast<std::string>()
<< ":";
for (auto& handle : pyself.attr("__dir__")()) {
std::string attribute = py::str(handle);
py::object member;
const py::str name = py::reinterpret_borrow<py::str>(handle);
py::object attribute;
try {
member = pyself.attr(attribute.c_str());
attribute = pyself.attr(name);
} catch (const std::exception& e) {
// Some properties are not valid for some uninitialized objects.
continue;
}
if (AttributeIsFunction(attribute, member)) {
if (AttributeIsFunction(name, attribute)) {
continue;
}
ss << "\n";
if (!after_subsummary) {
ss << prefix;
}
ss << attribute;
if (py::hasattr(member, "summary")) {
std::string summ = member.attr("summary")
ss << attribute.template cast<std::string>();
if (py::hasattr(attribute, "summary")) {
std::string summ = attribute.attr("summary")
.attr("__call__")(write_type)
.template cast<std::string>();
summ = std::regex_replace(summ, std::regex("\n"), "\n" + prefix);
ss << ": " << summ;
} else {
if (write_type) {
const std::string type_str =
py::str(py::type::of(member).attr("__name__"));
py::str(py::type::of(attribute).attr("__name__"));
ss << ": " << type_str;
after_subsummary = true;
}
std::string value = py::str(member);
if (value.length() > 80 && py::hasattr(member, "__len__")) {
const int length = member.attr("__len__")().template cast<int>();
std::string value = py::str(attribute);
if (value.length() > 80 && py::hasattr(attribute, "__len__")) {
const int length = attribute.attr("__len__")().template cast<int>();
value = StringPrintf(
"%c ... %d elements ... %c", value.front(), length, value.back());
}
Expand Down
Loading