Skip to content
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
33939b0
--more appropriately name json subgroup loading
jturner65 Aug 6, 2021
1d6c250
--change configuration to be backed by maps.
jturner65 Aug 6, 2021
d4e4280
--appropriately initialize maps for expected default vals.
jturner65 Aug 9, 2021
f541fb9
--Appropriately initialize dflt attr vals
jturner65 Aug 10, 2021
4309795
--appropriately set instance ids for non-registered attributes
jturner65 Aug 10, 2021
c12554d
--fix reading method for new configuration layout.
jturner65 Aug 10, 2021
aabc145
--fix typing for bindings.
jturner65 Aug 10, 2021
c7901e4
--modify python tests
jturner65 Aug 10, 2021
46dcee5
--repair rebase regression
jturner65 Aug 11, 2021
492ed3d
--fix/remove unsupported methods
jturner65 Aug 11, 2021
7c557fc
--clang-tidy suggestions
jturner65 Aug 11, 2021
f61eb5d
--remove float refs since python doesn't support c++ floats
jturner65 Aug 12, 2021
3621db0
--change user attributes setting to merging for physics objs
jturner65 Aug 12, 2021
916657b
--remove needless message spam
jturner65 Aug 12, 2021
da1d449
--restore and support untyped "get"
jturner65 Aug 12, 2021
1ba0bf9
--remove erroneous duplicate check.
jturner65 Aug 16, 2021
f5cb5de
--Implement storage via tagged union storage class
jturner65 Aug 16, 2021
81d4060
--clang-tidy
jturner65 Aug 16, 2021
35f31c3
--minor
jturner65 Aug 17, 2021
32108e7
--moved to separate PR
jturner65 Aug 17, 2021
91b13af
--minimize code dupe in ConfigValue
jturner65 Aug 17, 2021
04daae7
--Reviewer suggestions; Rename and bind storedType consts;
jturner65 Aug 17, 2021
b868a68
--provide binding access to create, edit and save subconfigurations
jturner65 Aug 17, 2021
70bda24
--add enum value representing unitialized state.
jturner65 Aug 18, 2021
2ff682c
--refer to subconfig consistently
jturner65 Aug 18, 2021
4f3a03f
--reviewer functionality requests; simplify config access
jturner65 Aug 18, 2021
834e020
--rename "get" binding to be more descriptive
jturner65 Aug 18, 2021
e652ec0
--return unknown type if key not found
jturner65 Aug 18, 2021
1bc7f44
--clang-tidy
jturner65 Aug 18, 2021
ed3bff4
-clang-tidy
jturner65 Aug 18, 2021
2c431ca
--added more tests : quaternion, subconfig access, key find
jturner65 Aug 18, 2021
d9d84e9
--more python tests
jturner65 Aug 18, 2021
a9decf0
--reviewer comments
jturner65 Aug 19, 2021
c8aa014
--missed tests
jturner65 Aug 19, 2021
f6d21d0
--change setOrMerge to be separate functions.
jturner65 Aug 20, 2021
d806839
--clang-tidy
jturner65 Aug 20, 2021
b39c920
--Add config namespace; redesign ConfigValue
jturner65 Aug 20, 2021
3a2f83d
--further progress on redesign
jturner65 Aug 20, 2021
379b5c8
--merge miss
jturner65 Aug 20, 2021
7a6ff8d
--reviewer suggestion.
jturner65 Aug 20, 2021
e915504
--clang-tidy
jturner65 Aug 20, 2021
41a993e
--fix debug stream; clang-tidy
jturner65 Aug 22, 2021
c0beac4
--more clang-tidy
jturner65 Aug 22, 2021
8057fa0
--bypass clang-tidy buggy checks - can't be const.
jturner65 Aug 23, 2021
985207e
--more specific nolint
jturner65 Aug 24, 2021
d13a2dc
--separate config namespace bindings from other core bindings
jturner65 Aug 24, 2021
f16d626
--another erroneous clan-tidy const complaint
jturner65 Aug 24, 2021
c7d31c5
--fix bug with copy/move; better func names; comments
jturner65 Aug 24, 2021
8d199c5
--support enum-to-descriptive name lookup for ConfigStoredTypes
jturner65 Aug 24, 2021
b03d0d0
--replace type-named getters with template.
jturner65 Aug 24, 2021
2738b4f
--add hash for ConfigStoredType
jturner65 Aug 24, 2021
d7d449d
--reviewer suggestion
jturner65 Aug 24, 2021
b53ab13
--remove superfluous/confusing template-to-user_config accessors
jturner65 Aug 24, 2021
8ba4ed8
--attempt to avoid type-punning warning.
jturner65 Aug 24, 2021
297579b
--minor binding fixes
jturner65 Aug 25, 2021
701f9e9
--more efficient subgroup add
jturner65 Aug 25, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@
" for k, _ in user_attributes_dict.items():\n",
" print(\n",
" \"Sphere Object user attribute : {} : {}\".format(\n",
" k, sphere_obj.user_attributes.get(k)\n",
" k, sphere_obj.user_attributes.get_as_string(k)\n",
" )\n",
" )\n",
"\n",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def simulate(sim, dt=1.0, get_frames=True):
for k, _ in user_attributes_dict.items():
print(
"Sphere Object user attribute : {} : {}".format(
k, sphere_obj.user_attributes.get(k)
k, sphere_obj.user_attributes.get_as_string(k)
)
)

Expand Down
2 changes: 1 addition & 1 deletion habitat_sim/bindings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from habitat_sim._ext.habitat_sim_bindings import (
CameraSensor,
CameraSensorSpec,
ConfigurationGroup,
Configuration,
CubeMapSensorBase,
CubeMapSensorBaseSpec,
EquirectangularSensor,
Expand Down
31 changes: 7 additions & 24 deletions src/esp/bindings/AttributesBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void initAttributesBindings(py::module& m) {

// ==== AbstractAttributes ====
py::class_<AbstractAttributes, esp::core::AbstractFileBasedManagedObject,
esp::core::Configuration, AbstractAttributes::ptr>(
esp::core::config::Configuration, AbstractAttributes::ptr>(
m, "AbstractAttributes")
.def(py::init(
&AbstractAttributes::create<const std::string&, const std::string&>))
Expand All @@ -65,29 +65,12 @@ void initAttributesBindings(py::module& m) {
"template_id", &AbstractAttributes::getID,
R"(System-generated ID for template. Will be unique among templates
of same type.)")
.def("get_user_config_bool",
&AbstractAttributes::getUserConfigValue<bool>)
.def("get_user_config_string",
&AbstractAttributes::getUserConfigValue<std::string>)
.def("get_user_config_int", &AbstractAttributes::getUserConfigValue<int>)
.def("get_user_config_double",
&AbstractAttributes::getUserConfigValue<double>)
.def("get_user_config_vec3",
&AbstractAttributes::getUserConfigValue<Magnum::Vector3>)
.def("get_user_config_quat",
&AbstractAttributes::getUserConfigValue<Magnum::Quaternion>)
.def("get_user_config_val",
&AbstractAttributes::getUserConfigValue<std::string>)
.def("set_user_config_val",
&AbstractAttributes::setUserConfigValue<std::string>)
.def("set_user_config_val", &AbstractAttributes::setUserConfigValue<int>)
.def("set_user_config_val",
&AbstractAttributes::setUserConfigValue<double>)
.def("set_user_config_val", &AbstractAttributes::setUserConfigValue<bool>)
.def("set_user_config_val",
&AbstractAttributes::setUserConfigValue<Magnum::Vector3>)
.def("set_user_config_val",
&AbstractAttributes::setUserConfigValue<Magnum::Quaternion>)

.def(
"get_user_config", &AbstractAttributes::editUserConfiguration,
R"(Retrieve the User Config object for this attributes, so that it can be
viewed or modified. Any changes to the user_config will require the owning
attributes to be re-registered.)")
.def_property_readonly(
"num_user_configs",
&AbstractAttributes::getNumUserDefinedConfigurations,
Expand Down
1 change: 1 addition & 0 deletions src/esp/bindings/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pybind11_add_module(
bindings.cpp
AttributesBindings.cpp
AttributesManagersBindings.cpp
ConfigBindings.cpp
CoreBindings.cpp
GeoBindings.cpp
GfxBindings.cpp
Expand Down
208 changes: 208 additions & 0 deletions src/esp/bindings/ConfigBindings.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
// Copyright (c) Facebook, Inc. and its affiliates.
// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

#include "esp/bindings/bindings.h"
#include "esp/core/Configuration.h"
namespace py = pybind11;
using py::literals::operator""_a;
using esp::logging::LoggingContext;

namespace esp {
namespace core {
namespace config {

void initConfigBindings(py::module& m) {
py::enum_<ConfigStoredType>(m, "ConfigStoredType")
.value("Unknown", ConfigStoredType::Unknown)
.value("Boolean", ConfigStoredType::Boolean)
.value("Integer", ConfigStoredType::Integer)
.value("Double", ConfigStoredType::Double)
.value("String", ConfigStoredType::String)
.value("MagnumVec3", ConfigStoredType::MagnumVec3)
.value("MagnumQuat", ConfigStoredType::MagnumQuat)
.value("MagnumRad", ConfigStoredType::MagnumRad);

py::class_<Configuration, Configuration::ptr>(m, "Configuration")
.def(py::init(&Configuration::create<>))

.def(
"get",
[](Configuration& self, const std::string& key) {
// switch on type
switch (self.getType(key)) {
case ConfigStoredType::Boolean:
return py::cast(self.get<bool>(key));
case ConfigStoredType::Integer:
return py::cast(self.get<int>(key));
case ConfigStoredType::Double:
return py::cast(self.get<double>(key));
case ConfigStoredType::String:
return py::cast(self.get<std::string>(key));
case ConfigStoredType::MagnumVec3:
return py::cast(self.get<Mn::Vector3>(key));
case ConfigStoredType::MagnumQuat:
return py::cast(self.get<Mn::Quaternion>(key));
case ConfigStoredType::MagnumRad:
return py::cast(self.get<Mn::Rad>(key));
default:
// unknown type or value not found
throw py::value_error("No valid value found for key " + key);
return py::cast(nullptr);
}
CORRADE_INTERNAL_ASSERT_UNREACHABLE();
},
R"(Retrieve the requested value, if it exists)")

.def("set", [](Configuration& self, const std::string& key,
const std::string& val) { self.set(key, val); })
.def("set", [](Configuration& self, const std::string& key,
const char* val) { self.set(key, val); })
.def("set", [](Configuration& self, const std::string& key,
const int val) { self.set(key, val); })
.def("set", [](Configuration& self, const std::string& key,
const double val) { self.set(key, val); })
.def("set", [](Configuration& self, const std::string& key,
const bool val) { self.set(key, val); })
.def("set", [](Configuration& self, const std::string& key,
const Magnum::Quaternion& val) { self.set(key, val); })
.def("set", [](Configuration& self, const std::string& key,
const Magnum::Vector3& val) { self.set(key, val); })

.def(
"get_type", &Configuration::getType,
R"(Retrieves the ConfigStoredType of the value referred to by the passed key.)")

.def(
"get_as_string", &Configuration::getAsString,
R"(Retrieves a string representation of the value referred to by the passed key.)")

.def("get_bool_keys",
&Configuration::getStoredKeys<ConfigStoredType::Boolean>)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Last thing, I don't feel getStoredKeys() needs to be templated, instead it could take the type as a parameter (getKeysByType() and since you have the ConfigStoredType exposed anyway, all these overloads could be just one get_keys_by_type() with the ConfigStoredType as a parameter.

.def("get_string_keys",
&Configuration::getStoredKeys<ConfigStoredType::String>)
.def("get_int_keys",
&Configuration::getStoredKeys<ConfigStoredType::Integer>)
.def("get_float_keys",
&Configuration::getStoredKeys<ConfigStoredType::Double>)
.def("get_vec3_keys",
&Configuration::getStoredKeys<ConfigStoredType::MagnumVec3>)
.def("get_quat_keys",
&Configuration::getStoredKeys<ConfigStoredType::MagnumQuat>)
.def("get_rad_keys",
&Configuration::getStoredKeys<ConfigStoredType::MagnumRad>)

.def("get_keys_and_types", &Configuration::getValueTypes,
R"(Returns a dictionary where the keys are the names of the values
this configuration holds and the values are the types of these values.)")
.def(
"get_subconfig_keys", &Configuration::getSubconfigKeys,
R"(Retrieves a list of the keys of this configuration's subconfigurations)")

.def("get_subconfig", &Configuration::editSubconfig,
pybind11::return_value_policy::reference,
R"(Get the subconfiguration with the given name.)", "name"_a)
.def("get_subconfig_copy", &Configuration::getSubconfigCopy,
pybind11::return_value_policy::reference,
R"(Get a copy of the subconfiguration with the given name.)",
"name"_a)
.def("save_subconfig", &Configuration::setSubconfigPtr,
R"(Save a subconfiguration with the given name.)", "name"_a,
"subconfig"_a)

.def(
"find_value_location", &Configuration::findValue,
R"(Returns a list of keys, in order, for the traversal of the nested subconfigurations in
this Configuration to get the requested key's value or subconfig. Key is not found if list is empty.)",
"key"_a)

.def(
"has_value", &Configuration::hasValue,
R"(Returns whether or not this Configuration has the passed key. Does not check subconfigurations.)",
"key"_a)

.def(
"has_bool",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here, there could be a single hasKeyOfType() / has_key_of_type(), taking a ConfigStoredType.

[](Configuration& self, const std::string& key) {
return self.checkMapForKeyAndType<bool>(key);
},
R"(Returns true if specified key references a boolean value in this configuration.)")
.def(
"has_int",
[](Configuration& self, const std::string& key) {
return self.checkMapForKeyAndType<int>(key);
},
R"(Returns true if specified key references a integer value in this configuration.)")
.def(
"has_string",
[](Configuration& self, const std::string& key) {
return self.checkMapForKeyAndType<std::string>(key);
},
R"(Returns true if specified key references a string value in this configuration.)")
.def(
"has_float",
[](Configuration& self, const std::string& key) {
return self.checkMapForKeyAndType<double>(key);
},
R"(Returns true if specified key references a float value in this configuration.)")
.def(
"has_quat",
[](Configuration& self, const std::string& key) {
return self.checkMapForKeyAndType<Mn::Quaternion>(key);
},
R"(Returns true if specified key references a Magnum::Quaternion value in this configuration.)")
.def(
"has_vec3",
[](Configuration& self, const std::string& key) {
return self.checkMapForKeyAndType<Mn::Vector3>(key);
},
R"(Returns true if specified key references a Magnum::Vector3 value in this configuration.)")
.def(
"has_rad",
[](Configuration& self, const std::string& key) {
return self.checkMapForKeyAndType<Mn::Rad>(key);
},
R"(Returns true if specified key references a Magnum::Rad value in this configuration.)")

.def(
"has_subconfig", &Configuration::hasSubconfig,
R"(Returns true if specified key references an existing subconfiguration within this configuration.)")

.def(
"remove",
[](Configuration& self, const std::string& key) {
// switch on type
switch (self.getType(key)) {
case ConfigStoredType::Boolean:
return py::cast(self.removeAndRetrieve<bool>(key));
case ConfigStoredType::Integer:
return py::cast(self.removeAndRetrieve<int>(key));
case ConfigStoredType::Double:
return py::cast(self.removeAndRetrieve<double>(key));
case ConfigStoredType::String:
return py::cast(self.removeAndRetrieve<std::string>(key));
case ConfigStoredType::MagnumVec3:
return py::cast(self.removeAndRetrieve<Mn::Vector3>(key));
case ConfigStoredType::MagnumQuat:
return py::cast(self.removeAndRetrieve<Mn::Quaternion>(key));
case ConfigStoredType::MagnumRad:
return py::cast(self.removeAndRetrieve<Mn::Rad>(key));
default:
// unknown type or value not found
throw py::value_error("No valid value found for key " + key);
return py::cast(nullptr);
}
CORRADE_INTERNAL_ASSERT_UNREACHABLE();
},
R"(Retrieve and remove the requested value, if it exists)")

.def(
"remove_subconfig", &Configuration::removeSubconfig,
R"(Removes and returns subconfiguration corresponding to passed key, if found. Gives warning otherwise.)");

} // initConfigBindings

} // namespace config
} // namespace core

} // namespace esp
22 changes: 0 additions & 22 deletions src/esp/bindings/CoreBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// LICENSE file in the root directory of this source tree.

#include "esp/bindings/bindings.h"
#include "esp/core/Configuration.h"
#include "esp/core/random.h"

namespace py = pybind11;
Expand All @@ -14,26 +13,6 @@ namespace esp {
namespace core {

void initCoreBindings(py::module& m) {
py::class_<Configuration, Configuration::ptr>(m, "ConfigurationGroup")
.def(py::init(&Configuration::create<>))
.def("get_bool", &Configuration::getBool)
.def("get_string", &Configuration::getString)
.def("get_int", &Configuration::getInt)
.def("get_double", &Configuration::getDouble)
.def("get_vec3", &Configuration::getVec3)
.def("get_quat", &Configuration::getQuat)
.def("get", &Configuration::getString)
.def("set", &Configuration::set<std::string>)
.def("set", &Configuration::set<int>)
.def("set", &Configuration::set<double>)
.def("set", &Configuration::set<bool>)
.def("set", &Configuration::set<Magnum::Vector3>)
.def("set", &Configuration::set<Magnum::Quaternion>)
.def("add_string_to_group", &Configuration::addStringToGroup)
.def("get_string_group", &Configuration::getStringGroup)
.def("has_value", &Configuration::hasValue)
.def("remove_value", &Configuration::removeValue);

// ==== struct RigidState ===
py::class_<RigidState, RigidState::ptr>(m, "RigidState")
.def(py::init(&RigidState::create<>))
Expand Down Expand Up @@ -70,7 +49,6 @@ void initCoreBindings(py::module& m) {
});
core.attr("_logging_context") = new LoggingContext{};
}

} // namespace core

} // namespace esp
1 change: 1 addition & 0 deletions src/esp/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ PYBIND11_MODULE(habitat_sim_bindings, m) {
// TODO(msb) gfx, scene, and sensor should not cross-depend
// TODO(msb) sim and sensor should not cross-depend
esp::initEspBindings(m);
esp::core::config::initConfigBindings(m);
esp::core::initCoreBindings(m);
esp::geo::initGeoBindings(m);
esp::scene::initSceneBindings(m);
Expand Down
3 changes: 3 additions & 0 deletions src/esp/bindings/bindings.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ namespace esp {
namespace core {

void initCoreBindings(pybind11::module& m);
namespace config {

void initConfigBindings(pybind11::module& m);
} // namespace config
} // namespace core

namespace geo {
Expand Down
1 change: 1 addition & 0 deletions src/esp/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ add_library(
Buffer.h
Check.cpp
Check.h
Configuration.cpp
Configuration.h
esp.cpp
esp.h
Expand Down
Loading