Skip to content

Commit

Permalink
Merge pull request #37196 from mantidproject/0-int-bounded-validator-…
Browse files Browse the repository at this point in the history
…not-working

IntBoundedValidator raises set attribute error
  • Loading branch information
SilkeSchomann committed Apr 30, 2024
2 parents e7add54 + c089110 commit 62692c2
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Mantid::Kernel::Logger g_log("Python Type Extractor");
namespace Mantid::PythonInterface {

struct PyNativeTypeExtractor {
using PythonOutputT = boost::make_recursive_variant<bool, long, double, std::string, Mantid::API::Workspace_sptr,
using PythonOutputT = boost::make_recursive_variant<bool, int, double, std::string, Mantid::API::Workspace_sptr,
std::vector<boost::recursive_variant_>>::type;

static PythonOutputT convert(const boost::python::object &obj) {
Expand All @@ -40,7 +40,7 @@ struct PyNativeTypeExtractor {
} else if (PyFloat_Check(rawptr)) {
out = extract<double>(obj);
} else if (PyLong_Check(rawptr)) {
out = extract<long>(obj);
out = static_cast<int>(extract<long>(obj));
} else if (PyUnicode_Check(rawptr)) {
out = extract<std::string>(obj);
} else if (auto extractor = extract<Mantid::API::Workspace_sptr>(obj); extractor.check()) {
Expand Down Expand Up @@ -79,13 +79,13 @@ class IPyTypeVisitor : public boost::static_visitor<> {

virtual ~IPyTypeVisitor() = default;
virtual void operator()(bool value) const = 0;
virtual void operator()(long value) const = 0;
virtual void operator()(int value) const = 0;
virtual void operator()(double value) const = 0;
virtual void operator()(std::string) const = 0;
virtual void operator()(Mantid::API::Workspace_sptr) const = 0;

virtual void operator()(std::vector<bool>) const = 0;
virtual void operator()(std::vector<long>) const = 0;
virtual void operator()(std::vector<int>) const = 0;
virtual void operator()(std::vector<double>) const = 0;
virtual void operator()(std::vector<std::string>) const = 0;

Expand All @@ -100,8 +100,8 @@ class IPyTypeVisitor : public boost::static_visitor<> {
applyVectorProp<bool>(values);
} else if (elemType == typeid(double)) {
applyVectorProp<double>(values);
} else if (elemType == typeid(long)) {
applyVectorProp<long>(values);
} else if (elemType == typeid(int)) {
applyVectorProp<int>(values);
} else if (elemType == typeid(std::string)) {
applyVectorProp<std::string>(values);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ class SetPropertyVisitor final : public Mantid::PythonInterface::IPyTypeVisitor
: m_alg(alg), m_propName(propName) {}

void operator()(bool value) const override { setProp(value); }
void operator()(long value) const override { setProp(static_cast<int>(value)); }
void operator()(int value) const override { setProp(value); }
void operator()(double value) const override { setProp(value); }
void operator()(std::string value) const override { m_alg->setPropertyValue(m_propName, value); }
void operator()(Mantid::API::Workspace_sptr ws) const override { m_alg->setProperty(m_propName, std::move(ws)); }

void operator()(std::vector<bool> value) const override { setProp(value); }
void operator()(std::vector<long> value) const override { setProp(value); }
void operator()(std::vector<int> value) const override { setProp(value); }
void operator()(std::vector<double> value) const override { setProp(value); }
void operator()(std::vector<std::string> value) const override { setProp(value); }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ class AttrVisitor : Mantid::PythonInterface::IPyTypeVisitor {
AttrVisitor(IFunction::Attribute &attrToUpdate) : m_attr(attrToUpdate) {}

void operator()(bool value) const override { m_attr.setValue(value); }
void operator()(long value) const override { m_attr.setValue(static_cast<int>(value)); }
void operator()(int value) const override { m_attr.setValue(value); }
void operator()(double value) const override { m_attr.setValue(value); }
void operator()(std::string value) const override { m_attr.setValue(std::move(value)); }
void operator()(Mantid::API::Workspace_sptr) const override { throw std::invalid_argument(m_errorMsg); }

void operator()(std::vector<bool>) const override { throw std::invalid_argument(m_errorMsg); }
void operator()(std::vector<long> value) const override {
void operator()(std::vector<int> value) const override {
// Previous existing code blindly converted any list type into a list of doubles.
// We now have to preserve this behaviour to maintain API compatibility as
// setValue only takes std::vector<double>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,5 @@ ArrayBoundedValidator<T> *createExclusiveArrayBoundedValidator(object lower = ob

void export_ArrayBoundedValidator() {
EXPORT_ARRAYBOUNDEDVALIDATOR(double, Float);
EXPORT_ARRAYBOUNDEDVALIDATOR(long, Int);
EXPORT_ARRAYBOUNDEDVALIDATOR(int, Int);
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,6 @@ namespace {

void export_ArrayLengthValidator() {
EXPORT_LENGTHVALIDATOR(double, Float);
EXPORT_LENGTHVALIDATOR(long, Int);
EXPORT_LENGTHVALIDATOR(int, Int);
EXPORT_LENGTHVALIDATOR(std::string, String);
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ template <typename TYPE> ArrayOrderedPairsValidator<TYPE> *createArrayOrderedPai
} // namespace
void export_ArrayOrderedPairsValidator() {
EXPORT_PAIRSVALIDATOR(double, Float);
EXPORT_PAIRSVALIDATOR(long, Int);
EXPORT_PAIRSVALIDATOR(int, Int);
}
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ ArrayProperty<T> *createArrayPropertyFromNDArray(const std::string &name, const
void export_ArrayProperty() {
// Match the python names to their C types
EXPORT_ARRAY_PROP(double, Float);
EXPORT_ARRAY_PROP(long, Int);
EXPORT_ARRAY_PROP(int, Int);
EXPORT_ARRAY_PROP(std::string, String);

// Needs these declarations also to ensure that properties not created in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,5 @@ BoundedValidator<T> *createExclusiveBoundedValidator(object lower = object(), ob

void export_BoundedValidator() {
EXPORT_BOUNDEDVALIDATOR(double, Float);
EXPORT_BOUNDEDVALIDATOR(long, Int);
EXPORT_BOUNDEDVALIDATOR(int, Int);
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@ template <typename T> ListValidator<T> *createListValidator(const boost::python:

void export_ListValidator() {
EXPORT_LISTVALIDATOR(std::string, String);
EXPORT_LISTVALIDATOR(long, Int);
EXPORT_LISTVALIDATOR(int, Int);
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ namespace {

void export_MandatoryValidator() {
EXPORT_MANDATORYVALIDATOR(double, Float);
EXPORT_MANDATORYVALIDATOR(long, Int);
EXPORT_MANDATORYVALIDATOR(int, Int);
EXPORT_MANDATORYVALIDATOR(std::string, String);

// Array types
EXPORT_MANDATORYVALIDATOR(std::vector<double>, FloatArray);
EXPORT_MANDATORYVALIDATOR(std::vector<long>, IntArray);
EXPORT_MANDATORYVALIDATOR(std::vector<int>, IntArray);
EXPORT_MANDATORYVALIDATOR(std::vector<std::string>, StringArray);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ void initTypeLookup(PyTypeIndex &index) {
using BoolHandler = TypedPropertyValueHandler<bool>;
index.emplace(&PyBool_Type, std::make_shared<BoolHandler>());

// Python 2/3 have an arbitrary-sized long type. The handler
// will raise an error if the input value overflows a C long
using IntHandler = TypedPropertyValueHandler<long>;
using IntHandler = TypedPropertyValueHandler<int>;
index.emplace(&PyLong_Type, std::make_shared<IntHandler>());

// In Python 3 all strings are unicode but in Python 2 unicode strings
Expand Down Expand Up @@ -89,8 +87,8 @@ void initArrayLookup(PyArrayIndex &index) {
using StringArrayHandler = SequenceTypeHandler<std::vector<std::string>>;
index.emplace("StringArray", std::make_shared<StringArrayHandler>());

using LongIntArrayHandler = SequenceTypeHandler<std::vector<long>>;
index.emplace("LongIntArray", std::make_shared<LongIntArrayHandler>());
using IntArrayHandler = SequenceTypeHandler<std::vector<int>>;
index.emplace("IntArray", std::make_shared<IntArrayHandler>());
}

/**
Expand Down Expand Up @@ -222,7 +220,7 @@ const std::string PropertyWithValueFactory::isArray(PyObject *const object) {
throw std::runtime_error("Unable to support extracting arrays of booleans.");
}
if (PyLong_Check(item)) {
return std::string("LongIntArray");
return std::string("IntArray");
}
GNU_DIAG_ON("parentheses-equality")
if (PyFloat_Check(item)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ class PropertyWithValueFactoryTest : public CxxTest::TestSuite {
checkPropertyValue<CType>(std::move(valueProp), pyvalue); \
}

void test_builtin_type_creates_int_type_property_without_error() {
testCreateSingleValueProperty<long>(FROM_INT(10));
}
void test_builtin_type_creates_int_type_property_without_error() { testCreateSingleValueProperty<int>(FROM_INT(10)); }

void test_builtin_type_creates_double_type_property_without_error() {
testCreateSingleValueProperty<double>(PyFloat_FromDouble(50.123));
Expand All @@ -67,11 +65,11 @@ class PropertyWithValueFactoryTest : public CxxTest::TestSuite {
}

void test_builtin_type_create_long_array_from_list_type_property() {
testCreateArrayProperty<long>(Py_BuildValue("[NN]", PyLong_FromLong(-10), PyLong_FromLong(4)));
testCreateArrayProperty<int>(Py_BuildValue("[NN]", PyLong_FromLong(-10), PyLong_FromLong(4)));
}

void test_builtin_type_create_int_array_from_list_type_property() {
testCreateArrayProperty<long>(Py_BuildValue("[ii]", -10, 4));
testCreateArrayProperty<int>(Py_BuildValue("[ii]", -10, 4));
}

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@
# Institut Laue - Langevin & CSNS, Institute of High Energy Physics, CAS
# SPDX - License - Identifier: GPL - 3.0 +
import unittest
from mantid.api import IFunction1D, IFunction, FunctionFactory
from mantid.kernel import StringListValidator, StringContainsValidator, FloatArrayBoundedValidator, FloatBoundedValidator
from mantid.api import IFunction1D
from mantid.kernel import (
FloatArrayBoundedValidator,
FloatBoundedValidator,
IntBoundedValidator,
StringContainsValidator,
StringListValidator,
)


class SimpleFuncWValidator(IFunction1D):
Expand All @@ -16,6 +22,8 @@ def category(self):
def init(self, input_case):
if input_case == "String List":
self.declareAttribute("StringAtt", "filename", StringListValidator(["filename", "test"]))
elif input_case == "Int Bounded":
self.declareAttribute("IntAtt", 3, IntBoundedValidator(lower=0, upper=3))
elif input_case == "Float Bounded":
self.declareAttribute("FloatAtt", 3.0, FloatBoundedValidator(0.0, 5.0))
elif input_case == "Array Bounded":
Expand All @@ -26,6 +34,8 @@ def init(self, input_case):
def invalid_init(self, input_case):
if input_case == "String List":
self.declareAttribute("StringAtt", "error", StringListValidator(["filename", "test"]))
elif input_case == "Int Bounded":
self.declareAttribute("IntAtt", 4, IntBoundedValidator(lower=0, upper=3))
elif input_case == "Float Bounded":
self.declareAttribute("FloatAtt", 10.0, FloatBoundedValidator(0.0, 5.0))
elif input_case == "Array Bounded":
Expand All @@ -38,6 +48,14 @@ def function1D(self, xvals):


class IFunction1DValidatorTest(unittest.TestCase):
def test_int_bounded_validator(self):
func = SimpleFuncWValidator()

self.assertRaises(Exception, func.invalid_init, "Int Bounded")
func.init("Int Bounded")

self.assertRaises(Exception, func.setAttributeValue, "IntAtt", 4)

def test_float_bounded_validator(self):
func = SimpleFuncWValidator()

Expand Down

0 comments on commit 62692c2

Please sign in to comment.