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

Expose AnyParameter map to C++ API and extend in SWIG #4479

Merged
merged 16 commits into from Jan 26, 2019
2 changes: 1 addition & 1 deletion src/interfaces/python/swig_typemaps.i
Expand Up @@ -1417,7 +1417,7 @@ def _internal_get_param(self, name):
except Exception:
raise
if name in self.parameter_names():
raise ValueError("The current Python API does not have a getter for '{}'".format(name))
raise ValueError("The current Python API does not have a getter for '{}' of type '{}'".format(name, self.parameter_type(name)))
else:
Copy link
Member

Choose a reason for hiding this comment

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

“Of type” maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

yup, I had that at first but then for some reason sounded weird, but I agree that is better!

raise KeyError("There is no parameter called '{}' in {}".format(name, self.get_name()))

Expand Down
39 changes: 37 additions & 2 deletions src/interfaces/swig/SGBase.i
@@ -1,3 +1,9 @@
/*
* This software is distributed under BSD 3-clause license (see LICENSE file).
*
* Authors: Gil Hoben, Heiko Strathmann, Sergey Lisitsyn
*/

/* base includes required by any module */
%include "stdint.i"
%include "std_string.i"
Expand Down Expand Up @@ -373,12 +379,39 @@ namespace std {
%include <shogun/base/Parallel.h>
%include <shogun/lib/StoppableSGObject.h>

#ifdef SWIGPYTHON
namespace shogun
{

%extend CSGObject
{
std::vector<std::string> parameter_names() const {
std::vector<std::string> result;
for (auto const& each: $self->get_params()) {
result.push_back(each.first);
}
return result;
}

std::string parameter_type(const std::string& name) const {
auto params = $self->get_params();
if (params.find(name) != params.end()) {
return params[name].get()->get_value().type();
}
else {
SG_SERROR("There is no parameter called '%s' in %s", name.c_str(), $self->get_name());
}
}

std::string parameter_description(const std::string& name) const {
auto params = $self->get_params();
if (params.find(name) != params.end()) {
return params[name].get()->get_properties().get_description();
}
else {
SG_SERROR("There is no parameter called '%s' in %s", name.c_str(), $self->get_name());
}
}

#ifdef SWIGPYTHON
std::string __str__() const
{
return $self->to_string();
Expand Down Expand Up @@ -485,9 +518,11 @@ namespace shogun
}

/*int getbuffer(PyObject *obj, Py_buffer *view, int flags) { return 0; }*/
#endif //SWIGPYTHON
}
}

#ifdef SWIGPYTHON
%pythoncode %{
try:
import copy_reg
Expand Down
30 changes: 20 additions & 10 deletions src/shogun/base/SGObject.cpp
Expand Up @@ -975,14 +975,16 @@ std::string CSGObject::to_string() const
return ss.str();
}

std::vector<std::string> CSGObject::parameter_names() const
#ifndef SWIG // SWIG should skip this part
std::map<std::string, std::shared_ptr<const AnyParameter>> CSGObject::get_params() const
{
std::vector<std::string> result;
std::transform(self->map.cbegin(), self->map.cend(), std::back_inserter(result),
// FIXME: const auto& each fails on gcc 4.8.4
[](const std::pair<BaseTag, AnyParameter>& each) -> std::string { return each.first.name(); });
std::map<std::string, std::shared_ptr<const AnyParameter>> result;
for (auto const& each: self->map) {
result.emplace(each.first.name(), std::make_shared<const AnyParameter>(each.second));
}
return result;
}
#endif

bool CSGObject::equals(const CSGObject* other) const
{
Expand Down Expand Up @@ -1068,11 +1070,19 @@ CSGObject* CSGObject::get(const std::string& name)
if (auto* result = get_sgobject_type_dispatcher<CLabels>(name))
return result;


SG_ERROR(
"Cannot get parameter %s::%s of type %s as object, not object type.\n",
get_name(), name.c_str(),
self->map[BaseTag(name)].get_value().type().c_str());
if (self->map.find(BaseTag(name)) != self->map.end())
{
SG_ERROR(
"Cannot get parameter %s::%s of type %s as object, not a shogun "
"object base type.\n",
get_name(), name.c_str(),
self->map[BaseTag(name)].get_value().type().c_str());
}
else
{
SG_ERROR(
"There is no parameter '%s' in %s.\n", name.c_str(), get_name());
}
return nullptr;
}

Expand Down
16 changes: 9 additions & 7 deletions src/shogun/base/SGObject.h
Expand Up @@ -28,6 +28,7 @@

#include <utility>
#include <vector>
#include <map>

/** \namespace shogun
* @brief all of classes and functions are contained in the shogun namespace
Expand Down Expand Up @@ -79,14 +80,13 @@ template <class T> class SGStringList;
#define VARARG_IMPL(base, count, ...) VARARG_IMPL2(base, count, __VA_ARGS__)
#define VARARG(base, ...) VARARG_IMPL(base, VA_NARGS(__VA_ARGS__), __VA_ARGS__)

#define SG_ADD3(param, name, description) \
#define SG_ADD3(param, name, description) \
{ \
this->m_parameters->add(param, name, description); \
this->watch_param( \
name, param, AnyParameterProperties()); \
this->watch_param(name, param, AnyParameterProperties(description)); \
}

#define SG_ADD4(param, name, description, param_properties) \
#define SG_ADD4(param, name, description, param_properties) \
{ \
AnyParameterProperties pprop = \
AnyParameterProperties(description, param_properties); \
Expand Down Expand Up @@ -533,11 +533,13 @@ class CSGObject
*/
virtual std::string to_string() const;

/** Returns set of all parameter names of the object.
/** Returns map of parameter names and AnyParameter pairs
* of the object.
*
*/
std::vector<std::string> parameter_names() const;

#ifndef SWIG // SWIG should skip this part
std::map<std::string, std::shared_ptr<const AnyParameter>> get_params() const;
Copy link
Member

Choose a reason for hiding this comment

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

doxygen string would be good, like what is this method doing.

Copy link
Member Author

@gf712 gf712 Jan 24, 2019

Choose a reason for hiding this comment

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

Oh it's already there but I put the #ifndef SWIG bellow it. I guess it should go above the doxygen string?

Copy link
Member

Choose a reason for hiding this comment

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

ah sorry I didnt see

#endif
/** Specializes a provided object to the specified type.
* Throws exception if the object cannot be specialized.
*
Expand Down
7 changes: 6 additions & 1 deletion tests/unit/base/SGObjectAll_unittest.cc
Expand Up @@ -293,7 +293,12 @@ TEST(SGObjectAll, DISABLED_tag_coverage)
std::vector<std::string> old_names;
for (auto i : range(obj->m_parameters->get_num_parameters()))
old_names.push_back(obj->m_parameters->get_parameter(i)->m_name);
auto tag_names = obj->parameter_names();

std::vector<std::string> tag_names;
std::transform(obj->get_params().cbegin(), obj->get_params().cend(), std::back_inserter(tag_names),
[](const std::pair<std::string, std::shared_ptr<const AnyParameter>>& each) -> std::string {
return each.first;
});

// hack to increase readability of error messages
old_names.push_back("_Shogun class: " + class_name);
Expand Down