From 159298965c316b5c3e8e3554a8555307ace3ec2e Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Wed, 5 Oct 2022 16:31:13 -0400 Subject: [PATCH 01/21] First try at improving type of metadata Signed-off-by: Jean-Christophe Morin --- .../otio_anyDictionary.cpp | 62 +++++++++++++++++++ .../otio_serializableObjects.cpp | 27 +++++++- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp index 385fd70db..08a259ca1 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp @@ -16,6 +16,68 @@ using namespace pybind11::literals; #include "opentimelineio/serializableObject.h" #include "opentimelineio/stringUtils.h" +// namespace pybind11 { namespace detail { +// template<> struct type_caster { +// public: +// /** +// * This macro establishes the name 'AnyDictionaryProxy' in +// * function signatures and declares a local variable +// * 'value' of type inty +// */ +// PYBIND11_TYPE_CASTER(AnyDictionaryProxy, const_name("AnyDictionaryProxy")); + +// /** +// * Conversion part 1 (Python->C++): convert a PyObject into an AnyDictionaryProxy +// * instance or return false upon failure. The second argument +// * indicates whether implicit conversions should be applied. +// */ +// bool load(handle src, bool) { +// // Extract PyObject from handle +// PyObject *source = src.ptr(); + +// if (!PyDict_Check(source)) { +// return false; +// } + +// // Now try to convert into a C++ int +// // value.long_value = PyLong_AsLong(tmp); +// PyObject *key, *value; +// Py_ssize_t pos = 0; + +// while (PyDict_Next(source, &pos, &key, &value)) { +// long i = PyLong_AsLong(value); +// if (i == -1 && PyErr_Occurred()) { +// return -1; +// } + +// PyObject *o = PyLong_FromLong(i + 1); +// if (o == NULL) +// return -1; + +// if (PyDict_SetItem(source, key, o) < 0) { +// Py_DECREF(o); +// return -1; +// } + +// Py_DECREF(o); +// } + +// /// Ensure return code was OK (to avoid out-of-range errors etc) +// return !PyErr_Occurred(); +// } + +// /** +// * Conversion part 2 (C++ -> Python): convert an AnyDictionaryProxy instance into +// * a Python object. The second and third arguments are used to +// * indicate the return value policy and parent object (for +// * ``return_value_policy::reference_internal``) and are generally +// * ignored by implicit casters. +// */ +// // static handle cast(AnyDictionaryProxy src, return_value_policy /* policy */, handle /* parent */) { +// // return PyLong_FromLong(src.long_value); +// }; +// }} + void otio_any_dictionary_bindings(py::module m) { py::class_(m, "AnyDictionaryIterator") .def("__iter__", &AnyDictionaryProxy::Iterator::iter) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index e2821199c..a919c5c37 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -187,8 +187,31 @@ static void define_bases1(py::module m) { py::class_>(m, "SerializableObjectWithMetadata", py::dynamic_attr()) - .def(py::init([](std::string name, py::object metadata) { - return new SOWithMetadata(name, py_to_any_dictionary(metadata)); + .def(py::init([](std::string name, AnyDictionaryProxy* metadata) { + AnyDictionary d = metadata->fetch_any_dictionary(); + return new SOWithMetadata(name, d); + }), + py::arg_v("name"_a = std::string()), + py::arg_v("metadata"_a = py::none())) + .def(py::init([](std::string name, py::dict metadata) { + // AnyDictionary d = metadata.fetch_any_dictionary(); + + AnyDictionary* d = new AnyDictionary(); + PyObject *source = metadata.ptr(); + + PyObject *key, *value; + Py_ssize_t pos = 0; + + while (PyDict_Next(source, &pos, &key, &value)) { + if (!PyUnicode_Check(key)) { + throw py::key_error("Keys should be of type string"); + } + + const char* key_name = PyUnicode_AsUTF8(key); + (*d)[key_name] = value; + } + + return new SOWithMetadata(name, (*d)); }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::none())) From 7fb3aeb508fa5dd8313601e0466c9ae7b3aac086 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Wed, 5 Oct 2022 18:17:26 -0400 Subject: [PATCH 02/21] debug Signed-off-by: Jean-Christophe Morin --- src/opentimelineio/anyDictionary.h | 15 +++++++++++-- .../otio_anyDictionary.h | 7 +++++++ .../otio_serializableObjects.cpp | 21 +++++++------------ 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/opentimelineio/anyDictionary.h b/src/opentimelineio/anyDictionary.h index af5f5a29f..aa1137483 100644 --- a/src/opentimelineio/anyDictionary.h +++ b/src/opentimelineio/anyDictionary.h @@ -9,6 +9,7 @@ #include #include #include +#include namespace opentimelineio { namespace OPENTIMELINEIO_VERSION { @@ -34,19 +35,25 @@ class AnyDictionary : private std::map AnyDictionary() : map{} , _mutation_stamp{} - {} + { + std::cout << "Constructing AnyDictionary\n"; + } // to be safe, avoid brace-initialization so as to not trigger // list initialization behavior in older compilers: AnyDictionary(const AnyDictionary& other) : map(other) , _mutation_stamp{} - {} + { + std::cout << "Constructing AnyDictionary\n"; + } ~AnyDictionary() { + std::cout << "Destructing AnyDictionary\n"; if (_mutation_stamp) { + std::cout << " There was a mutation\n"; _mutation_stamp->stamp = -1; _mutation_stamp->any_dictionary = nullptr; } @@ -225,8 +232,10 @@ class AnyDictionary : private std::map ~MutationStamp() { + std::cout << "Destructing MutationStamp\n"; if (any_dictionary) { + std::cout << " There was an AnyDictionary attached\n"; any_dictionary->_mutation_stamp = nullptr; if (owning) { @@ -251,8 +260,10 @@ class AnyDictionary : private std::map MutationStamp* get_or_create_mutation_stamp() { + std::cout << "get_or_create_mutation_stamp\n"; if (!_mutation_stamp) { + std::cout << " Creating mutation\n"; _mutation_stamp = new MutationStamp(this); } return _mutation_stamp; diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h index c49ebe887..62920a827 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h @@ -8,10 +8,17 @@ #include "opentimelineio/anyDictionary.h" +#include + namespace py = pybind11; struct AnyDictionaryProxy : public AnyDictionary::MutationStamp { + AnyDictionaryProxy() { + std::cout << "Constructing AnyDictionaryProxy\n"; + } + ~AnyDictionaryProxy() { + std::cout << "Destructing AnyDictionaryProxy\n"; } using MutationStamp = AnyDictionary::MutationStamp; diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index a919c5c37..6cdf6bd8a 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -196,22 +196,17 @@ static void define_bases1(py::module m) { .def(py::init([](std::string name, py::dict metadata) { // AnyDictionary d = metadata.fetch_any_dictionary(); - AnyDictionary* d = new AnyDictionary(); - PyObject *source = metadata.ptr(); - - PyObject *key, *value; - Py_ssize_t pos = 0; - - while (PyDict_Next(source, &pos, &key, &value)) { - if (!PyUnicode_Check(key)) { - throw py::key_error("Keys should be of type string"); + py::print("Creating new AnyDictionary from Python"); + AnyDictionary d = AnyDictionary(); + for (auto &it : metadata) { + if (!py::isinstance(it.first)) { + throw py::key_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); } - - const char* key_name = PyUnicode_AsUTF8(key); - (*d)[key_name] = value; + d[py::cast(it.first)] = it.second; } - return new SOWithMetadata(name, (*d)); + py::print("Creating new SOWithMetadata"); + return new SOWithMetadata(name, d); }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::none())) From 8ae911b5aa6f2eb6f19767a587bb75920a40ef81 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Thu, 6 Oct 2022 12:17:26 -0400 Subject: [PATCH 03/21] tmp Signed-off-by: Jean-Christophe Morin --- .../otio_serializableObjects.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index 6cdf6bd8a..75e48d37d 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -193,20 +193,20 @@ static void define_bases1(py::module m) { }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::none())) - .def(py::init([](std::string name, py::dict metadata) { + .def(py::init([](std::string name, py::object metadata) { // AnyDictionary d = metadata.fetch_any_dictionary(); py::print("Creating new AnyDictionary from Python"); - AnyDictionary d = AnyDictionary(); - for (auto &it : metadata) { - if (!py::isinstance(it.first)) { - throw py::key_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); - } - d[py::cast(it.first)] = it.second; - } + // auto d = AnyDictionary(); + // for (auto &it : metadata) { + // if (!py::isinstance(it.first)) { + // throw py::key_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); + // } + // d[py::cast(it.first)] = it.second; + // } py::print("Creating new SOWithMetadata"); - return new SOWithMetadata(name, d); + return new SOWithMetadata(name, py_to_any_dictionary(metadata)); }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::none())) From 126e694c71f07c2a3604a6fa10d13151a2c4b52c Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Thu, 6 Oct 2022 14:24:07 -0400 Subject: [PATCH 04/21] Progress! Things are working ish Signed-off-by: Jean-Christophe Morin --- src/opentimelineio/anyDictionary.h | 15 +-- .../otio_anyDictionary.cpp | 62 ------------ .../otio_anyDictionary.h | 7 -- .../otio_serializableObjects.cpp | 29 +++--- .../opentimelineio-bindings/otio_utils.cpp | 95 +++++++++++++++++++ .../opentimelineio-bindings/otio_utils.h | 3 + tests/test_serializable_object.py | 8 ++ 7 files changed, 122 insertions(+), 97 deletions(-) diff --git a/src/opentimelineio/anyDictionary.h b/src/opentimelineio/anyDictionary.h index aa1137483..af5f5a29f 100644 --- a/src/opentimelineio/anyDictionary.h +++ b/src/opentimelineio/anyDictionary.h @@ -9,7 +9,6 @@ #include #include #include -#include namespace opentimelineio { namespace OPENTIMELINEIO_VERSION { @@ -35,25 +34,19 @@ class AnyDictionary : private std::map AnyDictionary() : map{} , _mutation_stamp{} - { - std::cout << "Constructing AnyDictionary\n"; - } + {} // to be safe, avoid brace-initialization so as to not trigger // list initialization behavior in older compilers: AnyDictionary(const AnyDictionary& other) : map(other) , _mutation_stamp{} - { - std::cout << "Constructing AnyDictionary\n"; - } + {} ~AnyDictionary() { - std::cout << "Destructing AnyDictionary\n"; if (_mutation_stamp) { - std::cout << " There was a mutation\n"; _mutation_stamp->stamp = -1; _mutation_stamp->any_dictionary = nullptr; } @@ -232,10 +225,8 @@ class AnyDictionary : private std::map ~MutationStamp() { - std::cout << "Destructing MutationStamp\n"; if (any_dictionary) { - std::cout << " There was an AnyDictionary attached\n"; any_dictionary->_mutation_stamp = nullptr; if (owning) { @@ -260,10 +251,8 @@ class AnyDictionary : private std::map MutationStamp* get_or_create_mutation_stamp() { - std::cout << "get_or_create_mutation_stamp\n"; if (!_mutation_stamp) { - std::cout << " Creating mutation\n"; _mutation_stamp = new MutationStamp(this); } return _mutation_stamp; diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp index 08a259ca1..385fd70db 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp @@ -16,68 +16,6 @@ using namespace pybind11::literals; #include "opentimelineio/serializableObject.h" #include "opentimelineio/stringUtils.h" -// namespace pybind11 { namespace detail { -// template<> struct type_caster { -// public: -// /** -// * This macro establishes the name 'AnyDictionaryProxy' in -// * function signatures and declares a local variable -// * 'value' of type inty -// */ -// PYBIND11_TYPE_CASTER(AnyDictionaryProxy, const_name("AnyDictionaryProxy")); - -// /** -// * Conversion part 1 (Python->C++): convert a PyObject into an AnyDictionaryProxy -// * instance or return false upon failure. The second argument -// * indicates whether implicit conversions should be applied. -// */ -// bool load(handle src, bool) { -// // Extract PyObject from handle -// PyObject *source = src.ptr(); - -// if (!PyDict_Check(source)) { -// return false; -// } - -// // Now try to convert into a C++ int -// // value.long_value = PyLong_AsLong(tmp); -// PyObject *key, *value; -// Py_ssize_t pos = 0; - -// while (PyDict_Next(source, &pos, &key, &value)) { -// long i = PyLong_AsLong(value); -// if (i == -1 && PyErr_Occurred()) { -// return -1; -// } - -// PyObject *o = PyLong_FromLong(i + 1); -// if (o == NULL) -// return -1; - -// if (PyDict_SetItem(source, key, o) < 0) { -// Py_DECREF(o); -// return -1; -// } - -// Py_DECREF(o); -// } - -// /// Ensure return code was OK (to avoid out-of-range errors etc) -// return !PyErr_Occurred(); -// } - -// /** -// * Conversion part 2 (C++ -> Python): convert an AnyDictionaryProxy instance into -// * a Python object. The second and third arguments are used to -// * indicate the return value policy and parent object (for -// * ``return_value_policy::reference_internal``) and are generally -// * ignored by implicit casters. -// */ -// // static handle cast(AnyDictionaryProxy src, return_value_policy /* policy */, handle /* parent */) { -// // return PyLong_FromLong(src.long_value); -// }; -// }} - void otio_any_dictionary_bindings(py::module m) { py::class_(m, "AnyDictionaryIterator") .def("__iter__", &AnyDictionaryProxy::Iterator::iter) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h index 62920a827..c49ebe887 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h @@ -8,17 +8,10 @@ #include "opentimelineio/anyDictionary.h" -#include - namespace py = pybind11; struct AnyDictionaryProxy : public AnyDictionary::MutationStamp { - AnyDictionaryProxy() { - std::cout << "Constructing AnyDictionaryProxy\n"; - } - ~AnyDictionaryProxy() { - std::cout << "Destructing AnyDictionaryProxy\n"; } using MutationStamp = AnyDictionary::MutationStamp; diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index 75e48d37d..05c262396 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -188,28 +188,27 @@ static void define_bases1(py::module m) { py::class_>(m, "SerializableObjectWithMetadata", py::dynamic_attr()) .def(py::init([](std::string name, AnyDictionaryProxy* metadata) { + py::print("AnyDictionaryProxy"); AnyDictionary d = metadata->fetch_any_dictionary(); return new SOWithMetadata(name, d); }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::none())) - .def(py::init([](std::string name, py::object metadata) { - // AnyDictionary d = metadata.fetch_any_dictionary(); - - py::print("Creating new AnyDictionary from Python"); - // auto d = AnyDictionary(); - // for (auto &it : metadata) { - // if (!py::isinstance(it.first)) { - // throw py::key_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); - // } - // d[py::cast(it.first)] = it.second; - // } - - py::print("Creating new SOWithMetadata"); - return new SOWithMetadata(name, py_to_any_dictionary(metadata)); + .def(py::init([](std::string name, py::dict metadata) { + py::print("py::dict"); + AnyDictionary d = AnyDictionary(); + for (auto &it : metadata) { + if (!py::isinstance(it.first)) { + throw py::value_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); + } + + d[py::cast(it.first)] = py_to_any2(it.second); + } + + return new SOWithMetadata(name, d); }), py::arg_v("name"_a = std::string()), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property_readonly("metadata", [](SOWithMetadata* s) { auto ptr = s->metadata().get_or_create_mutation_stamp(); return (AnyDictionaryProxy*)(ptr); }, py::return_value_policy::take_ownership) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index c327374e3..a01d265af 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -105,6 +105,58 @@ static void py_to_any(py::object const& o, any* result) { result->swap(_value_to_any(o).cast()->a); } +any py_to_any2(py::handle const& o) { + py::type pytype = py::type::of(o); + if (o.ptr() == nullptr || o.is_none()) { + return any(nullptr); + } + + if (py::isinstance(o)) { + return any(o.cast()); + } + + if (py::isinstance(o)) { + try { + return any(o.cast()); + } catch (...) {} + + try { + return any(o.cast()); + } catch (...) {} + + try { + return any(o.cast()); + } catch (...) {} + + try { + return any(o.cast()); + } catch (...) {} + + throw std::runtime_error("Failed to convert Python int to C++ int"); + } + + if (py::isinstance(o)) { + return any(o.cast()); + } + + if (py::isinstance(o)) { + return any(o.cast()); + } + + if (py::isinstance(o)) { + AnyVector av; + for (auto &it : o) { + av.push_back(py_to_any2(it)); + } + return any(av); + } + + if (py::isinstance(o)) { + // TODO + } + throw py::value_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); +} + AnyDictionary py_to_any_dictionary(py::object const& o) { if (o.is_none()) { return AnyDictionary(); @@ -120,6 +172,49 @@ AnyDictionary py_to_any_dictionary(py::object const& o) { return safely_cast_any_dictionary_any(a); } +AnyDictionary pydict_to_any_dictionary(py::dict const& o) { + if (o.is_none()) { + return AnyDictionary(); + } + + any a; + py_to_any(o, &a); + if (!compare_typeids(a.type(), typeid(AnyDictionary))) { + throw py::type_error(string_printf("Expected an AnyDictionary (i.e. metadata); got %s instead", + type_name_for_error_message(a).c_str())); + } + + return safely_cast_any_dictionary_any(a); +} + +std::vector py_to_so_vector(pybind11::object const& o) { + if (_value_to_so_vector.is_none()) { + py::object core = py::module::import("opentimelineio.core"); + _value_to_so_vector = core.attr("_value_to_so_vector"); + } + + std::vector result; + if (o.is_none()) { + return result; + } + + /* + * We're depending on _value_to_so_vector(), written in Python, to + * not screw up, or we're going to crash. (1) It has to give us + * back an AnyVector. (2) Every element has to be a + * SerializableObject::Retainer<>. + */ + + py::object obj_vector = _value_to_so_vector(o); // need to retain this here or we'll lose the any... + AnyVector const& v = temp_safely_cast_any_vector_any(obj_vector.cast()->a); + + result.reserve(v.size()); + for (auto e: v) { + result.push_back(safely_cast_retainer_any(e)); + } + return result; +} + py::object any_to_py(any const& a, bool top_level) { std::type_info const& tInfo = a.type(); auto e = _py_cast_dispatch_table.find(&tInfo); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h index 2a979aabc..81bfa1788 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h @@ -152,6 +152,9 @@ struct PyAny { pybind11::object any_to_py(any const& a, bool top_level = false); pybind11::object plain_string(std::string const& s); pybind11::object plain_int(int i); +any py_to_any2(pybind11::handle const& o); AnyDictionary py_to_any_dictionary(pybind11::object const& o); +AnyDictionary pydict_to_any_dictionary(pybind11::dict const& o); +std::vector py_to_so_vector(pybind11::object const& o); bool compare_typeids(std::type_info const& lhs, std::type_info const& rhs); diff --git a/tests/test_serializable_object.py b/tests/test_serializable_object.py index 788a3f798..655d1d16c 100755 --- a/tests/test_serializable_object.py +++ b/tests/test_serializable_object.py @@ -36,6 +36,14 @@ def test_cons(self): so.metadata['foo'] = 'bar' self.assertEqual(so.metadata['foo'], 'bar') + def test_cons2(self): + so = otio.core.SerializableObjectWithMetadata(metadata={'key1': 'myvalue', 'key2': -999999999999, 'key3': [1, 2.5, 'asd']}) + so.metadata['foo'] = 'bar' + self.assertEqual(so.metadata['foo'], 'bar') + self.assertEqual(so.metadata['key1'], 'myvalue') + self.assertEqual(so.metadata['key2'], -999999999999) + self.assertEqual(list(so.metadata['key3']), [1, 2.5, 'asd']) # AnyVector. Is this right? + def test_update(self): so = otio.core.SerializableObjectWithMetadata() so.metadata.update({"foo": "bar"}) From 0866d9f6ee56f1e3ac643099097ba6fb44a9d0ca Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Thu, 6 Oct 2022 15:05:29 -0400 Subject: [PATCH 05/21] more types (crashing) Signed-off-by: Jean-Christophe Morin --- .../opentimelineio-bindings/otio_utils.cpp | 21 ++++++++++++++++- tests/test_serializable_object.py | 23 ++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index a01d265af..6f063b065 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -143,6 +143,10 @@ any py_to_any2(py::handle const& o) { return any(o.cast()); } + if (py::isinstance(o)) { + return any(((AnyVectorProxy*)o.ptr())->fetch_any_vector()); + } + if (py::isinstance(o)) { AnyVector av; for (auto &it : o) { @@ -151,8 +155,23 @@ any py_to_any2(py::handle const& o) { return any(av); } + if (py::isinstance(o)) { + return any(((AnyDictionaryProxy*)o.ptr())->fetch_any_dictionary()); + } + if (py::isinstance(o)) { - // TODO + AnyDictionary d; + + py::dict pyd = o.cast(); + for (auto &it : pyd) { + if (!py::isinstance(it.first)) { + throw py::value_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); + } + + d[it.first.cast()] = any(py_to_any2(it.second)); + } + + return any(d); } throw py::value_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); } diff --git a/tests/test_serializable_object.py b/tests/test_serializable_object.py index 655d1d16c..783c4e4fc 100755 --- a/tests/test_serializable_object.py +++ b/tests/test_serializable_object.py @@ -4,6 +4,7 @@ # Copyright Contributors to the OpenTimelineIO project import opentimelineio as otio +import opentimelineio._otio import opentimelineio.test_utils as otio_test_utils import unittest @@ -37,12 +38,32 @@ def test_cons(self): self.assertEqual(so.metadata['foo'], 'bar') def test_cons2(self): - so = otio.core.SerializableObjectWithMetadata(metadata={'key1': 'myvalue', 'key2': -999999999999, 'key3': [1, 2.5, 'asd']}) + v = opentimelineio._otio.AnyVector() + v.append(1) + v.append('inside any vector') + + d = opentimelineio._otio.AnyDictionary() + d['key_1'] = 1234 + d['key_2'] = {'asdasdasd': 5.6} + so = otio.core.SerializableObjectWithMetadata( + metadata={ + 'key1': 'myvalue', + 'key2': -999999999999, + 'key3': [1, 2.5, 'asd'], + 'key4': {'map1': [345]}, + 'key5': v, + 'key6': 123 + } + ) so.metadata['foo'] = 'bar' self.assertEqual(so.metadata['foo'], 'bar') self.assertEqual(so.metadata['key1'], 'myvalue') self.assertEqual(so.metadata['key2'], -999999999999) + self.assertIsInstance(so.metadata['key3'], opentimelineio._otio.AnyVector) self.assertEqual(list(so.metadata['key3']), [1, 2.5, 'asd']) # AnyVector. Is this right? + self.assertIsInstance(so.metadata['key4'], opentimelineio._otio.AnyDictionary) + # self.assertEqual(dict(so.metadata['key4']), {'map1': [345]}) + def test_update(self): so = otio.core.SerializableObjectWithMetadata() From 9009096dde864f5d3567dc00fc36e188ba3fee97 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Thu, 6 Oct 2022 17:01:59 -0400 Subject: [PATCH 06/21] Working state, but incomplete Signed-off-by: Jean-Christophe Morin --- .../otio_anyDictionary.h | 12 +++++++ .../opentimelineio-bindings/otio_anyVector.h | 12 +++++++ .../opentimelineio-bindings/otio_utils.cpp | 33 +++++++++++-------- tests/test_serializable_object.py | 29 ++++++++-------- 4 files changed, 60 insertions(+), 26 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h index c49ebe887..29ae2f883 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h @@ -11,6 +11,18 @@ namespace py = pybind11; struct AnyDictionaryProxy : public AnyDictionary::MutationStamp { + AnyDictionaryProxy() {} + AnyDictionaryProxy(const AnyDictionaryProxy& adp) { + AnyDictionary* ad = new AnyDictionary(); + + AnyDictionary::iterator ptr; + for (ptr = adp.any_dictionary->begin(); ptr != adp.any_dictionary->end(); ptr++) { + ad->insert(*ptr); + } + + any_dictionary = ad; + } + ~AnyDictionaryProxy() { } diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h index ed43e5dec..0f86d0973 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h @@ -13,6 +13,18 @@ namespace py = pybind11; struct AnyVectorProxy : public AnyVector::MutationStamp { using MutationStamp = AnyVector::MutationStamp; + AnyVectorProxy() {} + AnyVectorProxy(const AnyVectorProxy& avp) + { + AnyVector* av = new AnyVector(); + + AnyVector::iterator ptr; + for (ptr = avp.any_vector->begin(); ptr < avp.any_vector->end(); ptr++) { + av->push_back(*ptr); + } + any_vector = av; + } + static void throw_array_was_deleted() { throw py::value_error("Underlying C++ AnyVector object has been destroyed"); } diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index 6f063b065..d047d0715 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -143,23 +143,15 @@ any py_to_any2(py::handle const& o) { return any(o.cast()); } - if (py::isinstance(o)) { - return any(((AnyVectorProxy*)o.ptr())->fetch_any_vector()); - } - - if (py::isinstance(o)) { - AnyVector av; - for (auto &it : o) { - av.push_back(py_to_any2(it)); - } - return any(av); - } - + // Convert AnyDictionaryProxy and dict before vector and sequence because + // a dict is a sequence. if (py::isinstance(o)) { - return any(((AnyDictionaryProxy*)o.ptr())->fetch_any_dictionary()); + py::print("Converting AnyDictionaryProxy"); + return any(o.cast().fetch_any_dictionary()); } if (py::isinstance(o)) { + py::print("Converting py::dict"); AnyDictionary d; py::dict pyd = o.cast(); @@ -173,6 +165,21 @@ any py_to_any2(py::handle const& o) { return any(d); } + + if (py::isinstance(o)) { + py::print("Converting AnyVectorProxy"); + return any(o.cast().fetch_any_vector()); + } + + if (py::isinstance(o)) { + py::print("Converting py::sequence"); + AnyVector av; + for (auto &it : o) { + av.push_back(py_to_any2(it)); + } + return any(av); + } + throw py::value_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); } diff --git a/tests/test_serializable_object.py b/tests/test_serializable_object.py index 783c4e4fc..ea05cf0de 100755 --- a/tests/test_serializable_object.py +++ b/tests/test_serializable_object.py @@ -47,23 +47,26 @@ def test_cons2(self): d['key_2'] = {'asdasdasd': 5.6} so = otio.core.SerializableObjectWithMetadata( metadata={ - 'key1': 'myvalue', - 'key2': -999999999999, - 'key3': [1, 2.5, 'asd'], - 'key4': {'map1': [345]}, - 'key5': v, - 'key6': 123 + 'string': 'myvalue', + 'int': -999999999999, + 'list': [1, 2.5, 'asd'], + 'dict': {'map1': [345]}, + 'AnyVector': v, + 'AnyDictionary': d } ) so.metadata['foo'] = 'bar' self.assertEqual(so.metadata['foo'], 'bar') - self.assertEqual(so.metadata['key1'], 'myvalue') - self.assertEqual(so.metadata['key2'], -999999999999) - self.assertIsInstance(so.metadata['key3'], opentimelineio._otio.AnyVector) - self.assertEqual(list(so.metadata['key3']), [1, 2.5, 'asd']) # AnyVector. Is this right? - self.assertIsInstance(so.metadata['key4'], opentimelineio._otio.AnyDictionary) - # self.assertEqual(dict(so.metadata['key4']), {'map1': [345]}) - + self.assertEqual(so.metadata['string'], 'myvalue') + self.assertEqual(so.metadata['int'], -999999999999) + self.assertIsInstance(so.metadata['list'], opentimelineio._otio.AnyVector) + self.assertEqual(list(so.metadata['list']), [1, 2.5, 'asd']) # AnyVector. Is this right? + self.assertIsInstance(so.metadata['dict'], opentimelineio._otio.AnyDictionary) + # self.assertDictEqual(so.metadata['dict'], {'map1': [345]}) + self.assertIsInstance(so.metadata['AnyVector'], opentimelineio._otio.AnyVector) + self.assertEqual(list(so.metadata['AnyVector']), [1, 'inside any vector']) + self.assertIsInstance(so.metadata['AnyDictionary'], opentimelineio._otio.AnyDictionary) + self.assertEqual(dict(so.metadata['AnyDictionary']), {'key_1': 1234, 'key_2': {'asdasdasd': 5.6}}) def test_update(self): so = otio.core.SerializableObjectWithMetadata() From abd65045eb09968ebf1a1dd6b5849912ad7a6bc1 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Thu, 6 Oct 2022 19:09:09 -0400 Subject: [PATCH 07/21] Some notes about adding a custom caster for AnyDictionary Signed-off-by: Jean-Christophe Morin --- .../otio_serializableObjects.cpp | 3 + .../opentimelineio-bindings/otio_utils.h | 110 ++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index 05c262396..627a59965 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -194,6 +194,9 @@ static void define_bases1(py::module m) { }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::none())) + // TODO: We should use AnyDictionary and with the custom caster, we would no longer + // need py::object, py::dict or AnyDictionaryProxy to be exposed. + // https://stackoverflow.com/a/60744217 .def(py::init([](std::string name, py::dict metadata) { py::print("py::dict"); AnyDictionary d = AnyDictionary(); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h index 81bfa1788..7f2cf7595 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h @@ -17,6 +17,116 @@ using namespace opentimelineio::OPENTIMELINEIO_VERSION; void install_external_keepalive_monitor(SerializableObject* so, bool apply_now); +namespace pybind11 { namespace detail { + template struct type_caster> + : public optional_caster> {}; + + template<> struct type_caster + : public void_caster {}; + + template<> struct type_caster { + public: + /** + * This macro establishes the name 'any' in + * function signatures and declares a local variable + * 'value' of type any + */ + PYBIND11_TYPE_CASTER(AnyDictionary, const_name("Metadata[str, Union[bool, int, float, str, None, SerializableObject, RationalTime]]")); + + /** + * Conversion part 1 (Python->C++): convert a PyObject into an any + * instance or return false upon failure. The second argument + * indicates whether implicit conversions should be applied. + */ + bool load(handle src, bool) { + // TODO: Use what's in SOWithMetadata and otio_utils::py_to_any2. + // The idea is that we could simply use AnyDictionary as input parameters for + // metadata and this load method here would take care of basically + // taking the input and converting it to AnyDictionaryProxy? Or actually + // here we wuld simply take a dict and return AnyDictionary. + // Then in the cast method, it's where we would convert the AnyDictionary to + // and AnyDictionaryProxy. + + // if (pybind11::isinstance(src)) { + // bool result = src.cast(); + // value = create_safely_typed_any(std::move(result)); + // return true; + // } + + // if (pybind11::isinstance(src)) { + // int64_t result = src.cast(); + // value = create_safely_typed_any(std::move(result)); + // return true; + // } + + // if (pybind11::isinstance(src)) { + // double result = src.cast(); + // value = create_safely_typed_any(std::move(result)); + // return true; + // } + + // if (pybind11::isinstance(src)) { + // std::string result = src.cast(); + // value = create_safely_typed_any(std::move(result)); + // return true; + // } + + // if (src.is_none()) { + // value = any(); + // return true; + // } + + // if (pybind11::isinstance(src)) { + // // ((SerializableObject*)src.ptr()) + // // SerializableObject result = src.cast(); + // value = create_safely_typed_any(((SerializableObject*)src.ptr())); + // return true; + // } + + // if (pybind11::isinstance(src)) { + // RationalTime result = src.cast(); + // value = create_safely_typed_any(std::move(result)); + // return true; + // } + + // if (pybind11::isinstance(src)) { + // TimeRange result = src.cast(); + // value = create_safely_typed_any(std::move(result)); + // return true; + // } + + // if (pybind11::isinstance(src)) { + // TimeTransform result = src.cast(); + // value = create_safely_typed_any(std::move(result)); + // return true; + // } + + // if (pybind11::isinstance(src)) { + // AnyVectorProxy* result = src.cast(); + // value = create_safely_typed_any(std::move(result)); + // return true; + // } + return false; + } + + /** + * Conversion part 2 (C++ -> Python): convert an any instance into + * a Python object. The second and third arguments are used to + * indicate the return value policy and parent object (for + * ``return_value_policy::reference_internal``) and are generally + * ignored by implicit casters. + */ + static handle cast(any src, return_value_policy /* policy */, handle /* parent */) { + if (compare_typeids(src.type(), typeid(bool))) { + return pybind11::cast(safely_cast_bool_any(&src)); + } + + if (compare_typeids(src.type(), typeid(bool))) { + return pybind11::cast(safely_cast_bool_any(&src)); + } + } + }; +}} template struct managing_ptr { From 40a41f0d11f1a206b4c5e8c48066e611e4767122 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 15 Oct 2022 13:05:38 -0400 Subject: [PATCH 08/21] Little tweak in naming Signed-off-by: Jean-Christophe Morin --- .../opentimelineio-bindings/otio_anyDictionary.h | 11 ++++++----- .../opentimelineio-bindings/otio_anyVector.h | 10 +++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h index 29ae2f883..7170026c8 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h @@ -12,15 +12,16 @@ namespace py = pybind11; struct AnyDictionaryProxy : public AnyDictionary::MutationStamp { AnyDictionaryProxy() {} - AnyDictionaryProxy(const AnyDictionaryProxy& adp) { - AnyDictionary* ad = new AnyDictionary(); + AnyDictionaryProxy(const AnyDictionaryProxy& other) // Copy constructor. Required to convert a py::handle to an AnyDictionaryProxy. + { + AnyDictionary* d = new AnyDictionary; AnyDictionary::iterator ptr; - for (ptr = adp.any_dictionary->begin(); ptr != adp.any_dictionary->end(); ptr++) { - ad->insert(*ptr); + for (ptr = other.any_dictionary->begin(); ptr != other.any_dictionary->end(); ptr++) { + d->insert(*ptr); } - any_dictionary = ad; + any_dictionary = d; } ~AnyDictionaryProxy() { diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h index 0f86d0973..739d6b6de 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h @@ -14,15 +14,15 @@ struct AnyVectorProxy : public AnyVector::MutationStamp { using MutationStamp = AnyVector::MutationStamp; AnyVectorProxy() {} - AnyVectorProxy(const AnyVectorProxy& avp) + AnyVectorProxy(const AnyVectorProxy& other) // Copy constructor. Required to convert a py::handle to an AnyVectorProxy. { - AnyVector* av = new AnyVector(); + AnyVector* v = new AnyVector; AnyVector::iterator ptr; - for (ptr = avp.any_vector->begin(); ptr < avp.any_vector->end(); ptr++) { - av->push_back(*ptr); + for (ptr = other.any_vector->begin(); ptr < other.any_vector->end(); ptr++) { + v->push_back(*ptr); } - any_vector = av; + any_vector = v; } static void throw_array_was_deleted() { From acea32bef34ca12885f790b3595dadfb9e044232 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Fri, 4 Nov 2022 19:59:33 -0400 Subject: [PATCH 09/21] Fix bad rebase Signed-off-by: Jean-Christophe Morin --- .../opentimelineio-bindings/otio_utils.cpp | 28 ------------------- .../opentimelineio-bindings/otio_utils.h | 11 ++------ 2 files changed, 2 insertions(+), 37 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index d047d0715..69569a8c9 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -213,34 +213,6 @@ AnyDictionary pydict_to_any_dictionary(py::dict const& o) { return safely_cast_any_dictionary_any(a); } -std::vector py_to_so_vector(pybind11::object const& o) { - if (_value_to_so_vector.is_none()) { - py::object core = py::module::import("opentimelineio.core"); - _value_to_so_vector = core.attr("_value_to_so_vector"); - } - - std::vector result; - if (o.is_none()) { - return result; - } - - /* - * We're depending on _value_to_so_vector(), written in Python, to - * not screw up, or we're going to crash. (1) It has to give us - * back an AnyVector. (2) Every element has to be a - * SerializableObject::Retainer<>. - */ - - py::object obj_vector = _value_to_so_vector(o); // need to retain this here or we'll lose the any... - AnyVector const& v = temp_safely_cast_any_vector_any(obj_vector.cast()->a); - - result.reserve(v.size()); - for (auto e: v) { - result.push_back(safely_cast_retainer_any(e)); - } - return result; -} - py::object any_to_py(any const& a, bool top_level) { std::type_info const& tInfo = a.type(); auto e = _py_cast_dispatch_table.find(&tInfo); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h index 7f2cf7595..5511b9e10 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h @@ -17,13 +17,9 @@ using namespace opentimelineio::OPENTIMELINEIO_VERSION; void install_external_keepalive_monitor(SerializableObject* so, bool apply_now); -namespace pybind11 { namespace detail { - template struct type_caster> - : public optional_caster> {}; - - template<> struct type_caster - : public void_caster {}; +bool compare_typeids(std::type_info const& lhs, std::type_info const& rhs); +namespace pybind11 { namespace detail { template<> struct type_caster { public: /** @@ -265,6 +261,3 @@ pybind11::object plain_int(int i); any py_to_any2(pybind11::handle const& o); AnyDictionary py_to_any_dictionary(pybind11::object const& o); AnyDictionary pydict_to_any_dictionary(pybind11::dict const& o); -std::vector py_to_so_vector(pybind11::object const& o); - -bool compare_typeids(std::type_info const& lhs, std::type_info const& rhs); From a9de0f2ac6bf947516d47d8c2cf20207afe9bb4d Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 5 Nov 2022 18:17:08 -0400 Subject: [PATCH 10/21] Allow AnyDictionary creation from dict and AnyVector from sequence Signed-off-by: Jean-Christophe Morin --- .../otio_anyDictionary.cpp | 7 ++ .../otio_anyDictionary.h | 10 ++- .../otio_anyVector.cpp | 13 ++- .../opentimelineio-bindings/otio_anyVector.h | 4 + .../opentimelineio-bindings/otio_utils.cpp | 87 +++++++++++-------- .../opentimelineio-bindings/otio_utils.h | 10 ++- tests/test_serializable_object.py | 4 +- 7 files changed, 88 insertions(+), 47 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp index 385fd70db..52f5b6d21 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp @@ -23,6 +23,13 @@ void otio_any_dictionary_bindings(py::module m) { py::class_(m, "AnyDictionary") .def(py::init<>()) + .def(py::init([](py::dict item) { + AnyDictionary d = py_to_any3(item); + auto proxy = new AnyDictionaryProxy; + proxy->fetch_any_dictionary().swap(*d.get_or_create_mutation_stamp()->any_dictionary); + + return proxy; + })) .def("__getitem__", &AnyDictionaryProxy::get_item, "key"_a) .def("__internal_setitem__", &AnyDictionaryProxy::set_item, "key"_a, "item"_a) .def("__delitem__", &AnyDictionaryProxy::del_item, "key"_a) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h index 7170026c8..b363be95c 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h @@ -11,7 +11,15 @@ namespace py = pybind11; struct AnyDictionaryProxy : public AnyDictionary::MutationStamp { + using MutationStamp = AnyDictionary::MutationStamp; + AnyDictionaryProxy() {} + + // TODO: Should we instead just pass an AnyDictionary? + AnyDictionaryProxy(MutationStamp *d) { + any_dictionary = d->any_dictionary; + } + AnyDictionaryProxy(const AnyDictionaryProxy& other) // Copy constructor. Required to convert a py::handle to an AnyDictionaryProxy. { AnyDictionary* d = new AnyDictionary; @@ -26,8 +34,6 @@ struct AnyDictionaryProxy : public AnyDictionary::MutationStamp { ~AnyDictionaryProxy() { } - - using MutationStamp = AnyDictionary::MutationStamp; static void throw_dictionary_was_deleted() { throw py::value_error("Underlying C++ AnyDictionary has been destroyed"); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp index 4d974383b..0ffdf6333 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp @@ -19,6 +19,15 @@ void otio_any_vector_bindings(py::module m) { py::class_(m, "AnyVector") .def(py::init<>()) + .def(py::init([](const py::iterable &it) { + auto v = new AnyVector(); + for (py::handle h : it) { + v->push_back(py_to_any2(h)); + } + + AnyVectorProxy* a = new AnyVectorProxy(v->get_or_create_mutation_stamp()); + return a; + })) .def("__internal_getitem__", &AnyVectorProxy::get_item, "index"_a) .def("__internal_setitem__", &AnyVectorProxy::set_item, "index"_a, "item"_a) .def("__internal_delitem__", &AnyVectorProxy::del_item, "index"_a) @@ -26,7 +35,3 @@ void otio_any_vector_bindings(py::module m) { .def("__internal_insert", &AnyVectorProxy::insert) .def("__iter__", &AnyVectorProxy::iter, py::return_value_policy::reference_internal); } - - - - diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h index 739d6b6de..41ee2f343 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h @@ -14,6 +14,10 @@ struct AnyVectorProxy : public AnyVector::MutationStamp { using MutationStamp = AnyVector::MutationStamp; AnyVectorProxy() {} + AnyVectorProxy(MutationStamp *v) { + any_vector = v->any_vector; + } + AnyVectorProxy(const AnyVectorProxy& other) // Copy constructor. Required to convert a py::handle to an AnyVectorProxy. { AnyVector* v = new AnyVector; diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index 69569a8c9..2a166b8ed 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -106,41 +106,40 @@ static void py_to_any(py::object const& o, any* result) { } any py_to_any2(py::handle const& o) { - py::type pytype = py::type::of(o); if (o.ptr() == nullptr || o.is_none()) { return any(nullptr); } if (py::isinstance(o)) { - return any(o.cast()); + return any(py_to_any3(py::cast(o))); } if (py::isinstance(o)) { try { - return any(o.cast()); + return any(py_to_any3(py::cast(o))); } catch (...) {} try { - return any(o.cast()); + return any(py_to_any3(py::cast(o))); } catch (...) {} try { - return any(o.cast()); + return any(py_to_any3(py::cast(o))); } catch (...) {} try { - return any(o.cast()); + return any(py_to_any3(py::cast(o))); } catch (...) {} throw std::runtime_error("Failed to convert Python int to C++ int"); } if (py::isinstance(o)) { - return any(o.cast()); + return any(py_to_any3(py::cast(o))); } if (py::isinstance(o)) { - return any(o.cast()); + return any(py_to_any3(py::cast(o))); } // Convert AnyDictionaryProxy and dict before vector and sequence because @@ -151,19 +150,7 @@ any py_to_any2(py::handle const& o) { } if (py::isinstance(o)) { - py::print("Converting py::dict"); - AnyDictionary d; - - py::dict pyd = o.cast(); - for (auto &it : pyd) { - if (!py::isinstance(it.first)) { - throw py::value_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); - } - - d[it.first.cast()] = any(py_to_any2(it.second)); - } - - return any(d); + return any(py_to_any3(py::cast(o))); } if (py::isinstance(o)) { @@ -172,33 +159,57 @@ any py_to_any2(py::handle const& o) { } if (py::isinstance(o)) { - py::print("Converting py::sequence"); - AnyVector av; - for (auto &it : o) { - av.push_back(py_to_any2(it)); - } - return any(av); + return any(py_to_any3(py::cast(o))); } + py::type pytype = py::type::of(o); throw py::value_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); } -AnyDictionary py_to_any_dictionary(py::object const& o) { - if (o.is_none()) { - return AnyDictionary(); - } +bool py_to_any3(py::bool_ const& o) { + return o.cast(); +} - any a; - py_to_any(o, &a); - if (!compare_typeids(a.type(), typeid(AnyDictionary))) { - throw py::type_error(string_printf("Expected an AnyDictionary (i.e. metadata); got %s instead", - type_name_for_error_message(a).c_str())); +template +T py_to_any3(py::int_ const& o) { + return o.cast(); +} + +double py_to_any3(py::float_ const& o) { + return o.cast(); +} + +std::string py_to_any3(py::str const& o) { + return o.cast(); +} + +AnyDictionary py_to_any3(py::dict const& o) { + py::print("Converting py::dict"); + AnyDictionary d = AnyDictionary(); + + for (auto &it : o) { + if (!py::isinstance(it.first)) { + throw py::value_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); + } + + // Note that storing an any is expected, since AnyDictionary values + // can only be of type any. + d[it.first.cast()] = py_to_any2(it.second); } - return safely_cast_any_dictionary_any(a); + return d; } -AnyDictionary pydict_to_any_dictionary(py::dict const& o) { +AnyVector py_to_any3(py::iterable const& o) { + py::print("Converting py::sequence"); + AnyVector av = AnyVector(); + for (auto &it : o) { + av.push_back(py_to_any2(it)); + } + return av; +} + +AnyDictionary py_to_any_dictionary(py::object const& o) { if (o.is_none()) { return AnyDictionary(); } diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h index 5511b9e10..398660242 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h @@ -259,5 +259,13 @@ pybind11::object any_to_py(any const& a, bool top_level = false); pybind11::object plain_string(std::string const& s); pybind11::object plain_int(int i); any py_to_any2(pybind11::handle const& o); + +bool py_to_any3(pybind11::bool_ const& o); +template +T py_to_any3(pybind11::int_ const& o); +double py_to_any3(pybind11::float_ const& o); +std::string py_to_any3(pybind11::str const& o); +AnyDictionary py_to_any3(pybind11::dict const& o); +AnyVector py_to_any3(pybind11::iterable const& o); + AnyDictionary py_to_any_dictionary(pybind11::object const& o); -AnyDictionary pydict_to_any_dictionary(pybind11::dict const& o); diff --git a/tests/test_serializable_object.py b/tests/test_serializable_object.py index ea05cf0de..7233a7e3a 100755 --- a/tests/test_serializable_object.py +++ b/tests/test_serializable_object.py @@ -62,9 +62,9 @@ def test_cons2(self): self.assertIsInstance(so.metadata['list'], opentimelineio._otio.AnyVector) self.assertEqual(list(so.metadata['list']), [1, 2.5, 'asd']) # AnyVector. Is this right? self.assertIsInstance(so.metadata['dict'], opentimelineio._otio.AnyDictionary) - # self.assertDictEqual(so.metadata['dict'], {'map1': [345]}) + self.assertEqual(so.metadata['dict'], opentimelineio._otio.AnyDictionary({'map1': [345]})) self.assertIsInstance(so.metadata['AnyVector'], opentimelineio._otio.AnyVector) - self.assertEqual(list(so.metadata['AnyVector']), [1, 'inside any vector']) + self.assertEqual(list(so.metadata['AnyVector']), opentimelineio._otio.AnyVector([1, 'inside any vector'])) self.assertIsInstance(so.metadata['AnyDictionary'], opentimelineio._otio.AnyDictionary) self.assertEqual(dict(so.metadata['AnyDictionary']), {'key_1': 1234, 'key_2': {'asdasdasd': 5.6}}) From 27b9265e5b8f3640331541619da9c5ffee8604fc Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 5 Nov 2022 21:17:29 -0400 Subject: [PATCH 11/21] Completey removed py_to_any. No more roundtrip between C++ and Python to convert types Signed-off-by: Jean-Christophe Morin --- src/opentimelineio/serialization.cpp | 1 + .../otio_anyDictionary.cpp | 2 +- .../otio_anyVector.cpp | 2 +- .../otio_serializableObjects.cpp | 90 +++++++++---------- .../opentimelineio-bindings/otio_utils.cpp | 63 +++++-------- .../opentimelineio-bindings/otio_utils.h | 16 ++-- .../opentimelineio/adapters/fcp_xml.py | 2 +- 7 files changed, 81 insertions(+), 95 deletions(-) diff --git a/src/opentimelineio/serialization.cpp b/src/opentimelineio/serialization.cpp index 32f107deb..e6e4a8690 100644 --- a/src/opentimelineio/serialization.cpp +++ b/src/opentimelineio/serialization.cpp @@ -666,6 +666,7 @@ SerializableObject::Writer::_build_dispatch_tables() auto& wt = _write_dispatch_table; wt[&typeid(void)] = [this](any const&) { _encoder.write_null_value(); }; + wt[&typeid(nullptr)] = [this](any const&) { _encoder.write_null_value(); }; wt[&typeid(bool)] = [this](any const& value) { _encoder.write_value(any_cast(value)); }; diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp index 52f5b6d21..f5d18e9f3 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.cpp @@ -24,7 +24,7 @@ void otio_any_dictionary_bindings(py::module m) { py::class_(m, "AnyDictionary") .def(py::init<>()) .def(py::init([](py::dict item) { - AnyDictionary d = py_to_any3(item); + AnyDictionary d = py_to_cpp(item); auto proxy = new AnyDictionaryProxy; proxy->fetch_any_dictionary().swap(*d.get_or_create_mutation_stamp()->any_dictionary); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp index 0ffdf6333..458ecc148 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp @@ -22,7 +22,7 @@ void otio_any_vector_bindings(py::module m) { .def(py::init([](const py::iterable &it) { auto v = new AnyVector(); for (py::handle h : it) { - v->push_back(py_to_any2(h)); + v->push_back(py_to_any(h)); } AnyVectorProxy* a = new AnyVectorProxy(v->get_or_create_mutation_stamp()); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index 627a59965..44b36d9ec 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -188,7 +188,7 @@ static void define_bases1(py::module m) { py::class_>(m, "SerializableObjectWithMetadata", py::dynamic_attr()) .def(py::init([](std::string name, AnyDictionaryProxy* metadata) { - py::print("AnyDictionaryProxy"); + // py::print("AnyDictionaryProxy"); AnyDictionary d = metadata->fetch_any_dictionary(); return new SOWithMetadata(name, d); }), @@ -198,14 +198,14 @@ static void define_bases1(py::module m) { // need py::object, py::dict or AnyDictionaryProxy to be exposed. // https://stackoverflow.com/a/60744217 .def(py::init([](std::string name, py::dict metadata) { - py::print("py::dict"); + // py::print("py::dict"); AnyDictionary d = AnyDictionary(); for (auto &it : metadata) { if (!py::isinstance(it.first)) { throw py::value_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); } - d[py::cast(it.first)] = py_to_any2(it.second); + d[py::cast(it.first)] = py_to_any(it.second); } return new SOWithMetadata(name, d); @@ -234,7 +234,7 @@ The marked range may have a zero duration. The marked range is in the owning ite std::string name, TimeRange marked_range, std::string const& color, - py::object metadata) { + py::dict metadata) { return new Marker( name, marked_range, @@ -244,7 +244,7 @@ The marked range may have a zero duration. The marked range is in the owning ite py::arg_v("name"_a = std::string()), "marked_range"_a = TimeRange(), "color"_a = std::string(Marker::Color::red), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property("color", &Marker::color, &Marker::set_color, "Color string for this marker (for example: 'RED'), based on the :class:`~Color` enum.") .def_property("marked_range", &Marker::marked_range, &Marker::set_marked_range, "Range this marker applies to, relative to the :class:`.Item` this marker is attached to (e.g. the :class:`.Clip` or :class:`.Track` that owns this marker)."); @@ -278,13 +278,13 @@ a named collection. A :class:`~SerializableCollection` is useful for serializing multiple timelines, clips, or media references to a single file. )docstring") .def(py::init([](std::string const& name, optional> children, - py::object metadata) { + py::dict metadata) { return new SerializableCollection(name, vector_or_default(children), py_to_any_dictionary(metadata)); }), py::arg_v("name"_a = std::string()), "children"_a = py::none(), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def("__internal_getitem__", [](SerializableCollection* c, int index) { index = adjusted_vector_index(index, c->children()); if (index < 0 || index >= int(c->children().size())) { @@ -327,7 +327,7 @@ An object that can be composed within a :class:`~Composition` (such as :class:`~ py::class_>(m, "Item", py::dynamic_attr()) .def(py::init([](std::string name, optional source_range, - optional> effects, optional> markers, py::bool_ enabled, py::object metadata) { + optional> effects, optional> markers, py::bool_ enabled, py::dict metadata) { return new Item(name, source_range, py_to_any_dictionary(metadata), vector_or_default(effects), @@ -338,7 +338,7 @@ An object that can be composed within a :class:`~Composition` (such as :class:`~ "effects"_a = py::none(), "markers"_a = py::none(), "enabled"_a = true, - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property("enabled", &Item::enabled, &Item::set_enabled, "If true, an Item contributes to compositions. For example, when an audio/video clip is ``enabled=false`` the clip is muted/hidden.") .def_property("source_range", &Item::source_range, &Item::set_source_range) .def("available_range", [](Item* item) { @@ -379,7 +379,7 @@ An object that can be composed within a :class:`~Composition` (such as :class:`~ py::class_>(m, "Transition", py::dynamic_attr(), "Represents a transition between the two adjacent items in a :class:`.Track`. For example, a cross dissolve or wipe.") .def(py::init([](std::string const& name, std::string const& transition_type, RationalTime in_offset, RationalTime out_offset, - py::object metadata) { + py::dict metadata) { return new Transition(name, transition_type, in_offset, out_offset, py_to_any_dictionary(metadata)); }), @@ -387,7 +387,7 @@ An object that can be composed within a :class:`~Composition` (such as :class:`~ "transition_type"_a = std::string(), "in_offset"_a = RationalTime(), "out_offset"_a = RationalTime(), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property("transition_type", &Transition::transition_type, &Transition::set_transition_type, "Kind of transition, as defined by the :class:`Type` enum.") .def_property("in_offset", &Transition::in_offset, &Transition::set_in_offset, "Amount of the previous clip this transition overlaps, exclusive.") .def_property("out_offset", &Transition::out_offset, &Transition::set_out_offset, "Amount of the next clip this transition overlaps, exclusive.") @@ -412,7 +412,7 @@ Other effects are handled by the :class:`Effect` class. py::class_>(m, "Gap", py::dynamic_attr()) .def(py::init([](std::string name, TimeRange source_range, optional> effects, - optional> markers, py::object metadata) { + optional> markers, py::dict metadata) { return new Gap(source_range, name, vector_or_default(effects), vector_or_default(markers), @@ -421,9 +421,9 @@ Other effects are handled by the :class:`Effect` class. "source_range"_a = TimeRange(), "effects"_a = py::none(), "markers"_a = py::none(), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def(py::init([](std::string name, RationalTime duration, optional> effects, - optional> markers, py::object metadata) { + optional> markers, py::dict metadata) { return new Gap(duration, name, vector_or_default(effects), vector_or_default(markers), @@ -432,7 +432,7 @@ Other effects are handled by the :class:`Effect` class. "duration"_a = RationalTime(), "effects"_a = py::none(), "markers"_a = py::none(), - py::arg_v("metadata"_a = py::none())); + py::arg_v("metadata"_a = py::dict())); auto clip_class = py::class_>(m, "Clip", py::dynamic_attr(), R"docstring( A :class:`~Clip` is a segment of editable media (usually audio or video). @@ -440,14 +440,14 @@ A :class:`~Clip` is a segment of editable media (usually audio or video). Contains a :class:`.MediaReference` and a trim on that media reference. )docstring") .def(py::init([](std::string name, MediaReference* media_reference, - optional source_range, py::object metadata, + optional source_range, py::dict metadata, const std::string& active_media_reference) { return new Clip(name, media_reference, source_range, py_to_any_dictionary(metadata), active_media_reference); }), py::arg_v("name"_a = std::string()), "media_reference"_a = nullptr, "source_range"_a = nullopt, - py::arg_v("metadata"_a = py::none()), + py::arg_v("metadata"_a = py::dict()), "active_media_reference"_a = std::string(Clip::default_media_key)) .def_property_readonly_static("DEFAULT_MEDIA_KEY",[](py::object /* self */) { return Clip::default_media_key; @@ -473,7 +473,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u )docstring") .def(py::init([](std::string name, optional> children, - optional source_range, py::object metadata) { + optional source_range, py::dict metadata) { Composition* c = new Composition(name, source_range, py_to_any_dictionary(metadata)); c->set_children(vector_or_default(children), ErrorStatusHandler()); @@ -482,7 +482,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u py::arg_v("name"_a = std::string()), "children"_a = py::none(), "source_range"_a = nullopt, - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property_readonly("composition_kind", &Composition::composition_kind) .def("is_parent_of", &Composition::is_parent_of, "other"_a) .def("range_of_child_at_index", [](Composition* c, int index) { @@ -556,11 +556,11 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u composable_class .def(py::init([](std::string const& name, - py::object metadata) { + py::dict metadata) { return new Composable(name, py_to_any_dictionary(metadata)); }), py::arg_v("name"_a = std::string()), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def("parent", &Composable::parent) .def("visible", &Composable::visible) .def("overlapping", &Composable::overlapping); @@ -574,7 +574,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u track_class .def(py::init([](std::string name, optional> children, optional const& source_range, - std::string const& kind, py::object metadata) { + std::string const& kind, py::dict metadata) { auto composable_children = vector_or_default(children); Track* t = new Track( name, @@ -590,7 +590,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u "children"_a = py::none(), "source_range"_a = nullopt, "kind"_a = std::string(Track::Kind::video), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property("kind", &Track::kind, &Track::set_kind) .def("neighbors_of", [](Track* t, Composable* item, Track::NeighborGapPolicy policy) { auto result = t->neighbors_of(item, ErrorStatusHandler(), policy); @@ -611,7 +611,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u optional const& source_range, optional> markers, optional> effects, - py::object metadata) { + py::dict metadata) { auto composable_children = vector_or_default(children); Stack* s = new Stack( name, @@ -630,7 +630,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u "source_range"_a = nullopt, "markers"_a = py::none(), "effects"_a = py::none(), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def("find_clips", [](Stack* s, optional const& search_range, bool shallow_search) { return find_clips(s, search_range, shallow_search); }, "search_range"_a = nullopt, "shallow_search"_a = false); @@ -639,7 +639,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u .def(py::init([](std::string name, optional> children, optional global_start_time, - py::object metadata) { + py::dict metadata) { auto composable_children = vector_or_default(children); Timeline* t = new Timeline(name, global_start_time, py_to_any_dictionary(metadata)); @@ -650,7 +650,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u py::arg_v("name"_a = std::string()), "tracks"_a = py::none(), "global_start_time"_a = nullopt, - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property("global_start_time", &Timeline::global_start_time, &Timeline::set_global_start_time) .def_property("tracks", &Timeline::tracks, &Timeline::set_tracks) .def("duration", [](Timeline* t) { @@ -673,33 +673,33 @@ static void define_effects(py::module m) { py::class_>(m, "Effect", py::dynamic_attr()) .def(py::init([](std::string name, std::string effect_name, - py::object metadata) { + py::dict metadata) { return new Effect(name, effect_name, py_to_any_dictionary(metadata)); }), py::arg_v("name"_a = std::string()), "effect_name"_a = std::string(), - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property("effect_name", &Effect::effect_name, &Effect::set_effect_name); py::class_>(m, "TimeEffect", py::dynamic_attr(), "Base class for all effects that alter the timing of an item.") .def(py::init([](std::string name, std::string effect_name, - py::object metadata) { + py::dict metadata) { return new TimeEffect(name, effect_name, py_to_any_dictionary(metadata)); }), py::arg_v("name"_a = std::string()), "effect_name"_a = std::string(), - py::arg_v("metadata"_a = py::none())); + py::arg_v("metadata"_a = py::dict())); py::class_>(m, "LinearTimeWarp", py::dynamic_attr(), R"docstring( A time warp that applies a linear speed up or slow down across the entire clip. )docstring") .def(py::init([](std::string name, double time_scalar, - py::object metadata) { + py::dict metadata) { return new LinearTimeWarp(name, "LinearTimeWarp", time_scalar, py_to_any_dictionary(metadata)); }), py::arg_v("name"_a = std::string()), "time_scalar"_a = 1.0, - py::arg_v("metadata"_a = py::none())) + py::arg_v("metadata"_a = py::dict())) .def_property("time_scalar", &LinearTimeWarp::time_scalar, &LinearTimeWarp::set_time_scalar, R"docstring( Linear time scalar applied to clip. 2.0 means the clip occupies half the time in the parent item, i.e. plays at double speed, 0.5 means the clip occupies twice the time in the parent item, i.e. plays at half speed. @@ -709,10 +709,10 @@ Instead it affects the speed of the media displayed within that item. )docstring"); py::class_>(m, "FreezeFrame", py::dynamic_attr(), "Hold the first frame of the clip for the duration of the clip.") - .def(py::init([](std::string name, py::object metadata) { + .def(py::init([](std::string name, py::dict metadata) { return new FreezeFrame(name, py_to_any_dictionary(metadata)); }), py::arg_v("name"_a = std::string()), - py::arg_v("metadata"_a = py::none())); + py::arg_v("metadata"_a = py::dict())); } static void define_media_references(py::module m) { @@ -720,12 +720,12 @@ static void define_media_references(py::module m) { managing_ptr>(m, "MediaReference", py::dynamic_attr()) .def(py::init([](std::string name, optional available_range, - py::object metadata, + py::dict metadata, optional const& available_image_bounds) { return new MediaReference(name, available_range, py_to_any_dictionary(metadata), available_image_bounds); }), py::arg_v("name"_a = std::string()), "available_range"_a = nullopt, - py::arg_v("metadata"_a = py::none()), + py::arg_v("metadata"_a = py::dict()), "available_image_bounds"_a = nullopt) .def_property("available_range", &MediaReference::available_range, &MediaReference::set_available_range) @@ -736,7 +736,7 @@ static void define_media_references(py::module m) { managing_ptr>(m, "GeneratorReference", py::dynamic_attr()) .def(py::init([](std::string name, std::string generator_kind, optional const& available_range, - py::object parameters, py::object metadata, + py::object parameters, py::dict metadata, optional const& available_image_bounds) { return new GeneratorReference(name, generator_kind, available_range, @@ -747,7 +747,7 @@ static void define_media_references(py::module m) { "generator_kind"_a = std::string(), "available_range"_a = nullopt, "parameters"_a = py::none(), - py::arg_v("metadata"_a = py::none()), + py::arg_v("metadata"_a = py::dict()), "available_image_bounds"_a = nullopt) .def_property("generator_kind", &GeneratorReference::generator_kind, &GeneratorReference::set_generator_kind) .def_property_readonly("parameters", [](GeneratorReference* g) { @@ -764,7 +764,7 @@ Note that a :class:`~MissingReference` may have useful metadata, even if the loc .def(py::init([]( std::string name, optional available_range, - py::object metadata, + py::dict metadata, optional const& available_image_bounds) { return new MissingReference( name, @@ -774,7 +774,7 @@ Note that a :class:`~MissingReference` may have useful metadata, even if the loc }), py::arg_v("name"_a = std::string()), "available_range"_a = nullopt, - py::arg_v("metadata"_a = py::none()), + py::arg_v("metadata"_a = py::dict()), "available_image_bounds"_a = nullopt); @@ -782,7 +782,7 @@ Note that a :class:`~MissingReference` may have useful metadata, even if the loc managing_ptr>(m, "ExternalReference", py::dynamic_attr()) .def(py::init([](std::string target_url, optional const& available_range, - py::object metadata, + py::dict metadata, optional const& available_image_bounds) { return new ExternalReference(target_url, available_range, @@ -790,7 +790,7 @@ Note that a :class:`~MissingReference` may have useful metadata, even if the loc available_image_bounds); }), "target_url"_a = std::string(), "available_range"_a = nullopt, - py::arg_v("metadata"_a = py::none()), + py::arg_v("metadata"_a = py::dict()), "available_image_bounds"_a = nullopt) .def_property("target_url", &ExternalReference::target_url, &ExternalReference::set_target_url); @@ -878,7 +878,7 @@ Negative ``start_frame`` is also handled. The above example with a ``start_frame int frame_zero_padding, ImageSequenceReference::MissingFramePolicy const missing_frame_policy, optional const& available_range, - py::object metadata, + py::dict metadata, optional const& available_image_bounds) { return new ImageSequenceReference(target_url_base, name_prefix, @@ -900,7 +900,7 @@ Negative ``start_frame`` is also handled. The above example with a ``start_frame "frame_zero_padding"_a = 0, "missing_frame_policy"_a = ImageSequenceReference::MissingFramePolicy::error, "available_range"_a = nullopt, - py::arg_v("metadata"_a = py::none()), + py::arg_v("metadata"_a = py::dict()), "available_image_bounds"_a = nullopt) .def_property("target_url_base", &ImageSequenceReference::target_url_base, &ImageSequenceReference::set_target_url_base, "Everything leading up to the file name in the ``target_url``.") .def_property("name_prefix", &ImageSequenceReference::name_prefix, &ImageSequenceReference::set_name_prefix, "Everything in the file name leading up to the frame number.") diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index 2a166b8ed..182ccd460 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -52,6 +52,7 @@ void _build_any_to_py_dispatch_table() { auto& t = _py_cast_dispatch_table; t[&typeid(void)] = [](any const& /* a */, bool) { return py::none(); }; + t[&typeid(nullptr)] = [](any const& /* a */, bool) { return py::none(); }; t[&typeid(bool)] = [](any const& a, bool) { return py::cast(safely_cast_bool_any(a)); }; t[&typeid(int)] = [](any const& a, bool) { return plain_int(safely_cast_int_any(a)); }; t[&typeid(int64_t)] = [](any const& a, bool) { return plain_int(safely_cast_int64_any(a)); }; @@ -96,95 +97,86 @@ void _build_any_to_py_dispatch_table() { static py::object _value_to_any = py::none(); -static void py_to_any(py::object const& o, any* result) { - if (_value_to_any.is_none()) { - py::object core = py::module::import("opentimelineio.core"); - _value_to_any = core.attr("_value_to_any"); - } - - result->swap(_value_to_any(o).cast()->a); -} - -any py_to_any2(py::handle const& o) { +any py_to_any(py::handle const& o) { if (o.ptr() == nullptr || o.is_none()) { return any(nullptr); } if (py::isinstance(o)) { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } if (py::isinstance(o)) { try { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } catch (...) {} try { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } catch (...) {} try { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } catch (...) {} try { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } catch (...) {} throw std::runtime_error("Failed to convert Python int to C++ int"); } if (py::isinstance(o)) { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } if (py::isinstance(o)) { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } // Convert AnyDictionaryProxy and dict before vector and sequence because // a dict is a sequence. if (py::isinstance(o)) { - py::print("Converting AnyDictionaryProxy"); + // py::print("Converting AnyDictionaryProxy"); return any(o.cast().fetch_any_dictionary()); } if (py::isinstance(o)) { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } if (py::isinstance(o)) { - py::print("Converting AnyVectorProxy"); + // py::print("Converting AnyVectorProxy"); return any(o.cast().fetch_any_vector()); } if (py::isinstance(o)) { - return any(py_to_any3(py::cast(o))); + return any(py_to_cpp(py::cast(o))); } py::type pytype = py::type::of(o); throw py::value_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); } -bool py_to_any3(py::bool_ const& o) { +bool py_to_cpp(py::bool_ const& o) { return o.cast(); } template -T py_to_any3(py::int_ const& o) { +T py_to_cpp(py::int_ const& o) { return o.cast(); } -double py_to_any3(py::float_ const& o) { +double py_to_cpp(py::float_ const& o) { return o.cast(); } -std::string py_to_any3(py::str const& o) { +std::string py_to_cpp(py::str const& o) { return o.cast(); } -AnyDictionary py_to_any3(py::dict const& o) { - py::print("Converting py::dict"); +AnyDictionary py_to_cpp(py::dict const& o) { + // py::print("Converting py::dict"); AnyDictionary d = AnyDictionary(); for (auto &it : o) { @@ -194,17 +186,17 @@ AnyDictionary py_to_any3(py::dict const& o) { // Note that storing an any is expected, since AnyDictionary values // can only be of type any. - d[it.first.cast()] = py_to_any2(it.second); + d[it.first.cast()] = py_to_any(it.second); } return d; } -AnyVector py_to_any3(py::iterable const& o) { - py::print("Converting py::sequence"); +AnyVector py_to_cpp(py::iterable const& o) { + // py::print("Converting py::sequence"); AnyVector av = AnyVector(); for (auto &it : o) { - av.push_back(py_to_any2(it)); + av.push_back(py_to_any(it)); } return av; } @@ -214,14 +206,7 @@ AnyDictionary py_to_any_dictionary(py::object const& o) { return AnyDictionary(); } - any a; - py_to_any(o, &a); - if (!compare_typeids(a.type(), typeid(AnyDictionary))) { - throw py::type_error(string_printf("Expected an AnyDictionary (i.e. metadata); got %s instead", - type_name_for_error_message(a).c_str())); - } - - return safely_cast_any_dictionary_any(a); + return safely_cast_any_dictionary_any(py_to_any(o)); } py::object any_to_py(any const& a, bool top_level) { diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h index 398660242..a3f4f665e 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h @@ -35,7 +35,7 @@ namespace pybind11 { namespace detail { * indicates whether implicit conversions should be applied. */ bool load(handle src, bool) { - // TODO: Use what's in SOWithMetadata and otio_utils::py_to_any2. + // TODO: Use what's in SOWithMetadata and otio_utils::py_to_any. // The idea is that we could simply use AnyDictionary as input parameters for // metadata and this load method here would take care of basically // taking the input and converting it to AnyDictionaryProxy? Or actually @@ -258,14 +258,14 @@ struct PyAny { pybind11::object any_to_py(any const& a, bool top_level = false); pybind11::object plain_string(std::string const& s); pybind11::object plain_int(int i); -any py_to_any2(pybind11::handle const& o); +any py_to_any(pybind11::handle const& o); -bool py_to_any3(pybind11::bool_ const& o); +bool py_to_cpp(pybind11::bool_ const& o); template -T py_to_any3(pybind11::int_ const& o); -double py_to_any3(pybind11::float_ const& o); -std::string py_to_any3(pybind11::str const& o); -AnyDictionary py_to_any3(pybind11::dict const& o); -AnyVector py_to_any3(pybind11::iterable const& o); +T py_to_cpp(pybind11::int_ const& o); +double py_to_cpp(pybind11::float_ const& o); +std::string py_to_cpp(pybind11::str const& o); +AnyDictionary py_to_cpp(pybind11::dict const& o); +AnyVector py_to_cpp(pybind11::iterable const& o); AnyDictionary py_to_any_dictionary(pybind11::object const& o); diff --git a/src/py-opentimelineio/opentimelineio/adapters/fcp_xml.py b/src/py-opentimelineio/opentimelineio/adapters/fcp_xml.py index 34e420768..000c3a734 100644 --- a/src/py-opentimelineio/opentimelineio/adapters/fcp_xml.py +++ b/src/py-opentimelineio/opentimelineio/adapters/fcp_xml.py @@ -743,7 +743,7 @@ def track_for_element(self, track_element, track_kind, context): timeline_item_tags = {"clipitem", "generatoritem", "transitionitem"} md_dict = _xml_tree_to_dict(track_element, timeline_item_tags) - track_metadata = {META_NAMESPACE: md_dict} if md_dict else None + track_metadata = {META_NAMESPACE: md_dict} if md_dict else {} track = schema.Track( name=track_name, From cebb94522861ce020001749bc3dd8384a7e689c4 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 5 Nov 2022 21:39:19 -0400 Subject: [PATCH 12/21] Clean init of AnyVectorProxy Signed-off-by: Jean-Christophe Morin --- .../opentimelineio-bindings/otio_anyVector.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp index 458ecc148..f6db9074d 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp @@ -20,13 +20,10 @@ void otio_any_vector_bindings(py::module m) { py::class_(m, "AnyVector") .def(py::init<>()) .def(py::init([](const py::iterable &it) { - auto v = new AnyVector(); - for (py::handle h : it) { - v->push_back(py_to_any(h)); - } - - AnyVectorProxy* a = new AnyVectorProxy(v->get_or_create_mutation_stamp()); - return a; + auto v = py_to_cpp(it); + auto proxy = new AnyVectorProxy; + proxy->fetch_any_vector().swap(*v.get_or_create_mutation_stamp()->any_vector); + return proxy; })) .def("__internal_getitem__", &AnyVectorProxy::get_item, "index"_a) .def("__internal_setitem__", &AnyVectorProxy::set_item, "index"_a, "item"_a) From 2a53b9d4bb63915929be21f6f86c0b4e5716f01b Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 6 Nov 2022 16:18:29 -0500 Subject: [PATCH 13/21] Implement a working type caster that takes either an AnyDictionaryProxy or a py::dict Signed-off-by: Jean-Christophe Morin --- docs/conf.py | 36 +++++- .../otio_anyDictionary.h | 49 ++++++++ .../otio_serializableObjects.cpp | 19 ---- .../opentimelineio-bindings/otio_utils.h | 105 ------------------ .../opentimelineio/core/__init__.py | 15 +++ 5 files changed, 99 insertions(+), 125 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index f83edac93..29e8ea333 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -3,7 +3,12 @@ # Copyright Contributors to the OpenTimelineIO project import re +import docutils.nodes +import sphinx.addnodes +import sphinx.application +import sphinx.environment import sphinx_rtd_theme + import opentimelineio # -- Project information --------------------------------------------------------------- @@ -184,6 +189,35 @@ def process_docstring( lines[index] = line -def setup(app): +def process_missing_reference( + app: sphinx.application.Sphinx, + env: sphinx.environment.BuildEnvironment, + node: sphinx.addnodes.pending_xref, + contnode: docutils.nodes.Element +): + if node.get('refdomain') != 'py': + return None + + if node.get('reftype') == 'class': + reftarget = node.get('reftarget') + if reftarget == 'opentimelineio.core.Metadata': + # This is one heck of a hack. As it is right now, when opentimelineio.core.Metadata + # appears in a C++ function/method signature, it cannot be properly + # resolved by Sphinx. Not too sure why. + new_node = docutils.nodes.reference('') + new_node['refuri'] = f'{node.get("py:module")}.html#{reftarget}' + new_node['reftitle'] = reftarget + # new_node['classname'] = 'Metadata' + # Note that normally we would append "contnode" since it's + # the node that contains the text (opentimelineio.core.Metadata), + # but in our case we simply append a custom Text node so that the + # displayed is shorter (no module name). + new_node.append(docutils.nodes.Text('Metadata')) + + return new_node + + +def setup(app: sphinx.application.Sphinx): app.connect("autodoc-process-signature", process_signature) app.connect("autodoc-process-docstring", process_docstring) + app.connect("missing-reference", process_missing_reference) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h index b363be95c..5d1c26865 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h @@ -118,3 +118,52 @@ struct AnyDictionaryProxy : public AnyDictionary::MutationStamp { } }; +// Taken from https://github.com/pybind/pybind11/issues/1176#issuecomment-343312352 +// This is a custom type caster for the AnyDictionaryProxy class. This makes AnyDictionaryProxy +// accept both AnyDictionaryProxy and py::dict. +namespace pybind11 { namespace detail { + template <> struct type_caster : public type_caster_base { + using base = type_caster_base; + public: + + // Override the type reported in docstings. opentimelineio.core.Metadata is defined + // in Python. It's defined as a union of a dict and AnyDictionary. + // This will allow mypy to do its job. + static constexpr auto name = const_name("opentimelineio.core.Metadata"); + + /** + * Conversion part 1 (Python->C++): convert a PyObject into an any + * instance or return false upon failure. The second argument + * indicates whether implicit conversions should be applied. + */ + bool load(handle src, bool convert) { + // First try to convert using the base (default) type caster for AnyDictionaryProxy. + if (base::load(src, convert)) { + return true; + } + + // If we got a dict, then do our own thing to convert the dict into an AnyDictionaryProxy. + else if (py::isinstance(src)) { + auto proxy = new AnyDictionaryProxy(); + AnyDictionary&& d = py_to_cpp(py::cast(src)); + proxy->fetch_any_dictionary().swap(d); + value = proxy; + return true; + } + + return false; + } + + /** + * Conversion part 2 (C++ -> Python): convert an any instance into + * a Python object. The second and third arguments are used to + * indicate the return value policy and parent object (for + * ``return_value_policy::reference_internal``) and are generally + * ignored by implicit casters. + */ + // static handle cast(AnyDictionaryProxy *src, return_value_policy policy, handle parent) { + // /* Do any additional work here */ + // return base::cast(src, policy, parent); + // } + }; +}} diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index 44b36d9ec..f4e2e959a 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -188,29 +188,10 @@ static void define_bases1(py::module m) { py::class_>(m, "SerializableObjectWithMetadata", py::dynamic_attr()) .def(py::init([](std::string name, AnyDictionaryProxy* metadata) { - // py::print("AnyDictionaryProxy"); AnyDictionary d = metadata->fetch_any_dictionary(); return new SOWithMetadata(name, d); }), py::arg_v("name"_a = std::string()), - py::arg_v("metadata"_a = py::none())) - // TODO: We should use AnyDictionary and with the custom caster, we would no longer - // need py::object, py::dict or AnyDictionaryProxy to be exposed. - // https://stackoverflow.com/a/60744217 - .def(py::init([](std::string name, py::dict metadata) { - // py::print("py::dict"); - AnyDictionary d = AnyDictionary(); - for (auto &it : metadata) { - if (!py::isinstance(it.first)) { - throw py::value_error("Keys must be of type string, not " + py::cast(py::type::of(it.first).attr("__name__"))); - } - - d[py::cast(it.first)] = py_to_any(it.second); - } - - return new SOWithMetadata(name, d); - }), - py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::dict())) .def_property_readonly("metadata", [](SOWithMetadata* s) { auto ptr = s->metadata().get_or_create_mutation_stamp(); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h index a3f4f665e..2524854e6 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h @@ -19,111 +19,6 @@ void install_external_keepalive_monitor(SerializableObject* so, bool apply_now); bool compare_typeids(std::type_info const& lhs, std::type_info const& rhs); -namespace pybind11 { namespace detail { - template<> struct type_caster { - public: - /** - * This macro establishes the name 'any' in - * function signatures and declares a local variable - * 'value' of type any - */ - PYBIND11_TYPE_CASTER(AnyDictionary, const_name("Metadata[str, Union[bool, int, float, str, None, SerializableObject, RationalTime]]")); - - /** - * Conversion part 1 (Python->C++): convert a PyObject into an any - * instance or return false upon failure. The second argument - * indicates whether implicit conversions should be applied. - */ - bool load(handle src, bool) { - // TODO: Use what's in SOWithMetadata and otio_utils::py_to_any. - // The idea is that we could simply use AnyDictionary as input parameters for - // metadata and this load method here would take care of basically - // taking the input and converting it to AnyDictionaryProxy? Or actually - // here we wuld simply take a dict and return AnyDictionary. - // Then in the cast method, it's where we would convert the AnyDictionary to - // and AnyDictionaryProxy. - - // if (pybind11::isinstance(src)) { - // bool result = src.cast(); - // value = create_safely_typed_any(std::move(result)); - // return true; - // } - - // if (pybind11::isinstance(src)) { - // int64_t result = src.cast(); - // value = create_safely_typed_any(std::move(result)); - // return true; - // } - - // if (pybind11::isinstance(src)) { - // double result = src.cast(); - // value = create_safely_typed_any(std::move(result)); - // return true; - // } - - // if (pybind11::isinstance(src)) { - // std::string result = src.cast(); - // value = create_safely_typed_any(std::move(result)); - // return true; - // } - - // if (src.is_none()) { - // value = any(); - // return true; - // } - - // if (pybind11::isinstance(src)) { - // // ((SerializableObject*)src.ptr()) - // // SerializableObject result = src.cast(); - // value = create_safely_typed_any(((SerializableObject*)src.ptr())); - // return true; - // } - - // if (pybind11::isinstance(src)) { - // RationalTime result = src.cast(); - // value = create_safely_typed_any(std::move(result)); - // return true; - // } - - // if (pybind11::isinstance(src)) { - // TimeRange result = src.cast(); - // value = create_safely_typed_any(std::move(result)); - // return true; - // } - - // if (pybind11::isinstance(src)) { - // TimeTransform result = src.cast(); - // value = create_safely_typed_any(std::move(result)); - // return true; - // } - - // if (pybind11::isinstance(src)) { - // AnyVectorProxy* result = src.cast(); - // value = create_safely_typed_any(std::move(result)); - // return true; - // } - return false; - } - - /** - * Conversion part 2 (C++ -> Python): convert an any instance into - * a Python object. The second and third arguments are used to - * indicate the return value policy and parent object (for - * ``return_value_policy::reference_internal``) and are generally - * ignored by implicit casters. - */ - static handle cast(any src, return_value_policy /* policy */, handle /* parent */) { - if (compare_typeids(src.type(), typeid(bool))) { - return pybind11::cast(safely_cast_bool_any(&src)); - } - - if (compare_typeids(src.type(), typeid(bool))) { - return pybind11::cast(safely_cast_bool_any(&src)); - } - } - }; -}} - template struct managing_ptr { managing_ptr(T* ptr) diff --git a/src/py-opentimelineio/opentimelineio/core/__init__.py b/src/py-opentimelineio/opentimelineio/core/__init__.py index 5bd586cd4..7bc60d763 100644 --- a/src/py-opentimelineio/opentimelineio/core/__init__.py +++ b/src/py-opentimelineio/opentimelineio/core/__init__.py @@ -2,12 +2,18 @@ # Copyright Contributors to the OpenTimelineIO project """Core implementation details and wrappers around the C++ library""" +from __future__ import annotations + +from typing import Union, Dict, TypeAlias + +from .. opentime import RationalTime, TimeRange, TimeTransform from .. _otio import ( # noqa # errors CannotComputeAvailableRangeError, # classes + AnyDictionary, Composable, Composition, Item, @@ -32,6 +38,12 @@ release_to_schema_version_map, ) +Metadata: TypeAlias = Union[Dict[str, 'MetadataValue'], AnyDictionary] +"""OTIO custom metadata type.""" + +MetadataValue: TypeAlias = Union[bool, int, float, str, None, SerializableObject, RationalTime, TimeRange, TimeTransform, Metadata] +"""Metadata's values.""" + from . _core_utils import ( # noqa add_method, _value_to_any, @@ -46,6 +58,9 @@ ) __all__ = [ + 'Metadata', + 'MetadataValue', + 'AnyDictionary', 'Composable', 'Composition', 'Item', From f7fe99d2fa53d27c92b6bb34bfcd83ae35f48357 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 6 Nov 2022 16:19:03 -0500 Subject: [PATCH 14/21] Make py_to_any raise type_error Signed-off-by: Jean-Christophe Morin --- src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index 182ccd460..f0a0b647f 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -97,6 +97,7 @@ void _build_any_to_py_dispatch_table() { static py::object _value_to_any = py::none(); +// TODO: Missing RationalTime, TimeRange and TimeTransform any py_to_any(py::handle const& o) { if (o.ptr() == nullptr || o.is_none()) { return any(nullptr); @@ -123,7 +124,7 @@ any py_to_any(py::handle const& o) { return any(py_to_cpp(py::cast(o))); } catch (...) {} - throw std::runtime_error("Failed to convert Python int to C++ int"); + throw py::type_error("Failed to convert Python int to C++ int"); } if (py::isinstance(o)) { @@ -155,7 +156,7 @@ any py_to_any(py::handle const& o) { } py::type pytype = py::type::of(o); - throw py::value_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); + throw py::type_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); } bool py_to_cpp(py::bool_ const& o) { From 9c270aff40c7ec653cc39d6ae2bb2d8509b95aba Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 6 Nov 2022 16:39:45 -0500 Subject: [PATCH 15/21] Use AnyDictionaryProxy everywhere instead of py::object Signed-off-by: Jean-Christophe Morin --- .../otio_serializableObjects.cpp | 91 +++++++++---------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index f4e2e959a..be7104c9f 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -188,8 +188,7 @@ static void define_bases1(py::module m) { py::class_>(m, "SerializableObjectWithMetadata", py::dynamic_attr()) .def(py::init([](std::string name, AnyDictionaryProxy* metadata) { - AnyDictionary d = metadata->fetch_any_dictionary(); - return new SOWithMetadata(name, d); + return new SOWithMetadata(name, metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::dict())) @@ -215,12 +214,12 @@ The marked range may have a zero duration. The marked range is in the owning ite std::string name, TimeRange marked_range, std::string const& color, - py::dict metadata) { + AnyDictionaryProxy* metadata) { return new Marker( name, marked_range, color, - py_to_any_dictionary(metadata)); + metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), "marked_range"_a = TimeRange(), @@ -259,10 +258,10 @@ a named collection. A :class:`~SerializableCollection` is useful for serializing multiple timelines, clips, or media references to a single file. )docstring") .def(py::init([](std::string const& name, optional> children, - py::dict metadata) { + AnyDictionaryProxy*metadata) { return new SerializableCollection(name, vector_or_default(children), - py_to_any_dictionary(metadata)); }), + metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), "children"_a = py::none(), py::arg_v("metadata"_a = py::dict())) @@ -308,9 +307,9 @@ An object that can be composed within a :class:`~Composition` (such as :class:`~ py::class_>(m, "Item", py::dynamic_attr()) .def(py::init([](std::string name, optional source_range, - optional> effects, optional> markers, py::bool_ enabled, py::dict metadata) { + optional> effects, optional> markers, py::bool_ enabled, AnyDictionaryProxy* metadata) { return new Item(name, source_range, - py_to_any_dictionary(metadata), + metadata->fetch_any_dictionary(), vector_or_default(effects), vector_or_default(markers), enabled); }), @@ -360,10 +359,10 @@ An object that can be composed within a :class:`~Composition` (such as :class:`~ py::class_>(m, "Transition", py::dynamic_attr(), "Represents a transition between the two adjacent items in a :class:`.Track`. For example, a cross dissolve or wipe.") .def(py::init([](std::string const& name, std::string const& transition_type, RationalTime in_offset, RationalTime out_offset, - py::dict metadata) { + AnyDictionaryProxy* metadata) { return new Transition(name, transition_type, in_offset, out_offset, - py_to_any_dictionary(metadata)); }), + metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), "transition_type"_a = std::string(), "in_offset"_a = RationalTime(), @@ -393,22 +392,22 @@ Other effects are handled by the :class:`Effect` class. py::class_>(m, "Gap", py::dynamic_attr()) .def(py::init([](std::string name, TimeRange source_range, optional> effects, - optional> markers, py::dict metadata) { + optional> markers, AnyDictionaryProxy* metadata) { return new Gap(source_range, name, vector_or_default(effects), vector_or_default(markers), - py_to_any_dictionary(metadata)); }), + metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), "source_range"_a = TimeRange(), "effects"_a = py::none(), "markers"_a = py::none(), py::arg_v("metadata"_a = py::dict())) .def(py::init([](std::string name, RationalTime duration, optional> effects, - optional> markers, py::dict metadata) { + optional> markers, AnyDictionaryProxy* metadata) { return new Gap(duration, name, vector_or_default(effects), vector_or_default(markers), - py_to_any_dictionary(metadata)); }), + metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), "duration"_a = RationalTime(), "effects"_a = py::none(), @@ -421,9 +420,9 @@ A :class:`~Clip` is a segment of editable media (usually audio or video). Contains a :class:`.MediaReference` and a trim on that media reference. )docstring") .def(py::init([](std::string name, MediaReference* media_reference, - optional source_range, py::dict metadata, + optional source_range, AnyDictionaryProxy* metadata, const std::string& active_media_reference) { - return new Clip(name, media_reference, source_range, py_to_any_dictionary(metadata), active_media_reference); + return new Clip(name, media_reference, source_range, metadata->fetch_any_dictionary(), active_media_reference); }), py::arg_v("name"_a = std::string()), "media_reference"_a = nullptr, @@ -454,9 +453,9 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u )docstring") .def(py::init([](std::string name, optional> children, - optional source_range, py::dict metadata) { + optional source_range, AnyDictionaryProxy* metadata) { Composition* c = new Composition(name, source_range, - py_to_any_dictionary(metadata)); + metadata->fetch_any_dictionary()); c->set_children(vector_or_default(children), ErrorStatusHandler()); return c; }), @@ -537,8 +536,8 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u composable_class .def(py::init([](std::string const& name, - py::dict metadata) { - return new Composable(name, py_to_any_dictionary(metadata)); + AnyDictionaryProxy* metadata) { + return new Composable(name, metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::dict())) @@ -555,13 +554,13 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u track_class .def(py::init([](std::string name, optional> children, optional const& source_range, - std::string const& kind, py::dict metadata) { + std::string const& kind, AnyDictionaryProxy* metadata) { auto composable_children = vector_or_default(children); Track* t = new Track( name, source_range, kind, - py_to_any_dictionary(metadata) + metadata->fetch_any_dictionary() ); if (!composable_children.empty()) t->set_children(composable_children, ErrorStatusHandler()); @@ -592,12 +591,12 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u optional const& source_range, optional> markers, optional> effects, - py::dict metadata) { + AnyDictionaryProxy* metadata) { auto composable_children = vector_or_default(children); Stack* s = new Stack( name, source_range, - py_to_any_dictionary(metadata), + metadata->fetch_any_dictionary(), vector_or_default(effects), vector_or_default(markers) ); @@ -620,10 +619,10 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u .def(py::init([](std::string name, optional> children, optional global_start_time, - py::dict metadata) { + AnyDictionaryProxy* metadata) { auto composable_children = vector_or_default(children); Timeline* t = new Timeline(name, global_start_time, - py_to_any_dictionary(metadata)); + metadata->fetch_any_dictionary()); if (!composable_children.empty()) t->tracks()->set_children(composable_children, ErrorStatusHandler()); return t; @@ -654,8 +653,8 @@ static void define_effects(py::module m) { py::class_>(m, "Effect", py::dynamic_attr()) .def(py::init([](std::string name, std::string effect_name, - py::dict metadata) { - return new Effect(name, effect_name, py_to_any_dictionary(metadata)); }), + AnyDictionaryProxy* metadata) { + return new Effect(name, effect_name, metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), "effect_name"_a = std::string(), py::arg_v("metadata"_a = py::dict())) @@ -664,8 +663,8 @@ static void define_effects(py::module m) { py::class_>(m, "TimeEffect", py::dynamic_attr(), "Base class for all effects that alter the timing of an item.") .def(py::init([](std::string name, std::string effect_name, - py::dict metadata) { - return new TimeEffect(name, effect_name, py_to_any_dictionary(metadata)); }), + AnyDictionaryProxy* metadata) { + return new TimeEffect(name, effect_name, metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), "effect_name"_a = std::string(), py::arg_v("metadata"_a = py::dict())); @@ -675,9 +674,9 @@ A time warp that applies a linear speed up or slow down across the entire clip. )docstring") .def(py::init([](std::string name, double time_scalar, - py::dict metadata) { + AnyDictionaryProxy* metadata) { return new LinearTimeWarp(name, "LinearTimeWarp", time_scalar, - py_to_any_dictionary(metadata)); }), + metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), "time_scalar"_a = 1.0, py::arg_v("metadata"_a = py::dict())) @@ -690,8 +689,8 @@ Instead it affects the speed of the media displayed within that item. )docstring"); py::class_>(m, "FreezeFrame", py::dynamic_attr(), "Hold the first frame of the clip for the duration of the clip.") - .def(py::init([](std::string name, py::dict metadata) { - return new FreezeFrame(name, py_to_any_dictionary(metadata)); }), + .def(py::init([](std::string name, AnyDictionaryProxy* metadata) { + return new FreezeFrame(name, metadata->fetch_any_dictionary()); }), py::arg_v("name"_a = std::string()), py::arg_v("metadata"_a = py::dict())); } @@ -701,9 +700,9 @@ static void define_media_references(py::module m) { managing_ptr>(m, "MediaReference", py::dynamic_attr()) .def(py::init([](std::string name, optional available_range, - py::dict metadata, + AnyDictionaryProxy* metadata, optional const& available_image_bounds) { - return new MediaReference(name, available_range, py_to_any_dictionary(metadata), available_image_bounds); }), + return new MediaReference(name, available_range, metadata->fetch_any_dictionary(), available_image_bounds); }), py::arg_v("name"_a = std::string()), "available_range"_a = nullopt, py::arg_v("metadata"_a = py::dict()), @@ -717,17 +716,17 @@ static void define_media_references(py::module m) { managing_ptr>(m, "GeneratorReference", py::dynamic_attr()) .def(py::init([](std::string name, std::string generator_kind, optional const& available_range, - py::object parameters, py::dict metadata, + AnyDictionaryProxy* parameters, AnyDictionaryProxy* metadata, optional const& available_image_bounds) { return new GeneratorReference(name, generator_kind, available_range, - py_to_any_dictionary(parameters), - py_to_any_dictionary(metadata), + parameters->fetch_any_dictionary(), + metadata->fetch_any_dictionary(), available_image_bounds); }), py::arg_v("name"_a = std::string()), "generator_kind"_a = std::string(), "available_range"_a = nullopt, - "parameters"_a = py::none(), + "parameters"_a = py::dict(), py::arg_v("metadata"_a = py::dict()), "available_image_bounds"_a = nullopt) .def_property("generator_kind", &GeneratorReference::generator_kind, &GeneratorReference::set_generator_kind) @@ -745,12 +744,12 @@ Note that a :class:`~MissingReference` may have useful metadata, even if the loc .def(py::init([]( std::string name, optional available_range, - py::dict metadata, + AnyDictionaryProxy* metadata, optional const& available_image_bounds) { return new MissingReference( name, available_range, - py_to_any_dictionary(metadata), + metadata->fetch_any_dictionary(), available_image_bounds); }), py::arg_v("name"_a = std::string()), @@ -763,11 +762,11 @@ Note that a :class:`~MissingReference` may have useful metadata, even if the loc managing_ptr>(m, "ExternalReference", py::dynamic_attr()) .def(py::init([](std::string target_url, optional const& available_range, - py::dict metadata, + AnyDictionaryProxy* metadata, optional const& available_image_bounds) { return new ExternalReference(target_url, available_range, - py_to_any_dictionary(metadata), + metadata->fetch_any_dictionary(), available_image_bounds); }), "target_url"_a = std::string(), "available_range"_a = nullopt, @@ -859,7 +858,7 @@ Negative ``start_frame`` is also handled. The above example with a ``start_frame int frame_zero_padding, ImageSequenceReference::MissingFramePolicy const missing_frame_policy, optional const& available_range, - py::dict metadata, + AnyDictionaryProxy* metadata, optional const& available_image_bounds) { return new ImageSequenceReference(target_url_base, name_prefix, @@ -870,7 +869,7 @@ Negative ``start_frame`` is also handled. The above example with a ``start_frame frame_zero_padding, missing_frame_policy, available_range, - py_to_any_dictionary(metadata), + metadata->fetch_any_dictionary(), available_image_bounds); }), "target_url_base"_a = std::string(), "name_prefix"_a = std::string(), From c9e05d6c68b6142f7a02d955603820eb6171a3b4 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 4 Dec 2022 13:48:36 -0500 Subject: [PATCH 16/21] Add support for RationalTime, TimeRange and TimeTransform Signed-off-by: Jean-Christophe Morin --- .../opentimelineio-bindings/otio_utils.cpp | 24 ++++++++++++------ .../opentimelineio-bindings/otio_utils.h | 2 ++ tests/test_serializable_object.py | 25 ++++++++++++++++--- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index f0a0b647f..74d4d0ce3 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -95,9 +95,6 @@ void _build_any_to_py_dispatch_table() { } } -static py::object _value_to_any = py::none(); - -// TODO: Missing RationalTime, TimeRange and TimeTransform any py_to_any(py::handle const& o) { if (o.ptr() == nullptr || o.is_none()) { return any(nullptr); @@ -138,7 +135,6 @@ any py_to_any(py::handle const& o) { // Convert AnyDictionaryProxy and dict before vector and sequence because // a dict is a sequence. if (py::isinstance(o)) { - // py::print("Converting AnyDictionaryProxy"); return any(o.cast().fetch_any_dictionary()); } @@ -147,7 +143,6 @@ any py_to_any(py::handle const& o) { } if (py::isinstance(o)) { - // py::print("Converting AnyVectorProxy"); return any(o.cast().fetch_any_vector()); } @@ -155,6 +150,18 @@ any py_to_any(py::handle const& o) { return any(py_to_cpp(py::cast(o))); } + if (py::isinstance(o)) { + return any(py_to_cpp(o)); + } + + if (py::isinstance(o)) { + return any(py_to_cpp(o)); + } + + if (py::isinstance(o)) { + return any(py_to_cpp(o)); + } + py::type pytype = py::type::of(o); throw py::type_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); } @@ -177,7 +184,6 @@ std::string py_to_cpp(py::str const& o) { } AnyDictionary py_to_cpp(py::dict const& o) { - // py::print("Converting py::dict"); AnyDictionary d = AnyDictionary(); for (auto &it : o) { @@ -194,7 +200,6 @@ AnyDictionary py_to_cpp(py::dict const& o) { } AnyVector py_to_cpp(py::iterable const& o) { - // py::print("Converting py::sequence"); AnyVector av = AnyVector(); for (auto &it : o) { av.push_back(py_to_any(it)); @@ -202,6 +207,11 @@ AnyVector py_to_cpp(py::iterable const& o) { return av; } +template +T py_to_cpp(py::handle const& o) { + return o.cast(); +} + AnyDictionary py_to_any_dictionary(py::object const& o) { if (o.is_none()) { return AnyDictionary(); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h index 2524854e6..1229d6d53 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h @@ -162,5 +162,7 @@ double py_to_cpp(pybind11::float_ const& o); std::string py_to_cpp(pybind11::str const& o); AnyDictionary py_to_cpp(pybind11::dict const& o); AnyVector py_to_cpp(pybind11::iterable const& o); +template +T py_to_cpp(pybind11::handle const& o); AnyDictionary py_to_any_dictionary(pybind11::object const& o); diff --git a/tests/test_serializable_object.py b/tests/test_serializable_object.py index 7233a7e3a..cad97ce09 100755 --- a/tests/test_serializable_object.py +++ b/tests/test_serializable_object.py @@ -6,6 +6,7 @@ import opentimelineio as otio import opentimelineio._otio import opentimelineio.test_utils as otio_test_utils +import opentimelineio.opentime import unittest import json @@ -52,7 +53,16 @@ def test_cons2(self): 'list': [1, 2.5, 'asd'], 'dict': {'map1': [345]}, 'AnyVector': v, - 'AnyDictionary': d + 'AnyDictionary': d, + 'RationalTime': opentimelineio.opentime.RationalTime(value=10.0, rate=5.0), + 'TimeRange': opentimelineio.opentime.TimeRange( + opentimelineio.opentime.RationalTime(value=1.0), + opentimelineio.opentime.RationalTime(value=100.0) + ), + 'TimeTransform': opentimelineio.opentime.TimeTransform( + offset=opentimelineio.opentime.RationalTime(value=55.0), + scale=999 + ) } ) so.metadata['foo'] = 'bar' @@ -62,11 +72,20 @@ def test_cons2(self): self.assertIsInstance(so.metadata['list'], opentimelineio._otio.AnyVector) self.assertEqual(list(so.metadata['list']), [1, 2.5, 'asd']) # AnyVector. Is this right? self.assertIsInstance(so.metadata['dict'], opentimelineio._otio.AnyDictionary) - self.assertEqual(so.metadata['dict'], opentimelineio._otio.AnyDictionary({'map1': [345]})) + # self.assertEqual(so.metadata['dict'], opentimelineio._otio.AnyDictionary({'map1': [345]})) self.assertIsInstance(so.metadata['AnyVector'], opentimelineio._otio.AnyVector) - self.assertEqual(list(so.metadata['AnyVector']), opentimelineio._otio.AnyVector([1, 'inside any vector'])) + # self.assertEqual(list(so.metadata['AnyVector']), opentimelineio._otio.AnyVector([1, 'inside any vector'])) self.assertIsInstance(so.metadata['AnyDictionary'], opentimelineio._otio.AnyDictionary) self.assertEqual(dict(so.metadata['AnyDictionary']), {'key_1': 1234, 'key_2': {'asdasdasd': 5.6}}) + self.assertEqual(so.metadata['RationalTime'], opentimelineio.opentime.RationalTime(value=10.0, rate=5.0)) + self.assertEqual(so.metadata['TimeRange'], opentimelineio.opentime.TimeRange( + opentimelineio.opentime.RationalTime(value=1.0), + opentimelineio.opentime.RationalTime(value=100.0) + )) + self.assertEqual(so.metadata['TimeTransform'], opentimelineio.opentime.TimeTransform( + offset=opentimelineio.opentime.RationalTime(value=55.0), + scale=999 + )) def test_update(self): so = otio.core.SerializableObjectWithMetadata() From dc6e4d5fd884eac4d276d2ce576aa949ab45e2a2 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 4 Dec 2022 14:31:41 -0500 Subject: [PATCH 17/21] Add support to check equality of AnyVector Signed-off-by: Jean-Christophe Morin --- .../opentimelineio/core/_core_utils.py | 40 +++++++++++++++++++ tests/test_serializable_object.py | 9 +++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio/core/_core_utils.py b/src/py-opentimelineio/opentimelineio/core/_core_utils.py index df2a075b9..e437f3bab 100644 --- a/src/py-opentimelineio/opentimelineio/core/_core_utils.py +++ b/src/py-opentimelineio/opentimelineio/core/_core_utils.py @@ -302,6 +302,41 @@ def insert(self, index, item): if conversion_func else item ) + def __le__(self, other): # Taken from collections.abc.Set + if not isinstance(other, collections.abc.Sequence): + return NotImplemented + if len(self) > len(other): + return False + for elem in self: + if elem not in other: + return False + return True + + def __lt__(self, other): # Taken from collections.abc.Set + if not isinstance(other, collections.abc.Sequence): + return NotImplemented + return len(self) < len(other) and self.__le__(other) + + def __gt__(self, other): # Taken from collections.abc.Set + if not isinstance(other, collections.abc.Sequence): + return NotImplemented + return len(self) > len(other) and self.__ge__(other) + + def __ge__(self, other): # Taken from collections.abc.Set + if not isinstance(other, collections.abc.Sequence): + return NotImplemented + if len(self) < len(other): + return False + for elem in other: + if elem not in self: + return False + return True + + def __eq__(self, other): # Taken from collections.abc.Set + if not isinstance(other, collections.abc.Sequence): + return NotImplemented + return len(self) == len(other) and self.__le__(other) + collections.abc.MutableSequence.register(sequenceClass) sequenceClass.__radd__ = __radd__ sequenceClass.__add__ = __add__ @@ -311,6 +346,11 @@ def insert(self, index, item): sequenceClass.insert = insert sequenceClass.__str__ = __str__ sequenceClass.__repr__ = __repr__ + sequenceClass.__le__ = __le__ + sequenceClass.__lt__ = __lt__ + sequenceClass.__gt__ = __gt__ + sequenceClass.__ge__ = __ge__ + sequenceClass.__eq__ = __eq__ seen = set() for klass in (collections.abc.MutableSequence, collections.abc.Sequence): diff --git a/tests/test_serializable_object.py b/tests/test_serializable_object.py index cad97ce09..5d0da426d 100755 --- a/tests/test_serializable_object.py +++ b/tests/test_serializable_object.py @@ -70,13 +70,14 @@ def test_cons2(self): self.assertEqual(so.metadata['string'], 'myvalue') self.assertEqual(so.metadata['int'], -999999999999) self.assertIsInstance(so.metadata['list'], opentimelineio._otio.AnyVector) - self.assertEqual(list(so.metadata['list']), [1, 2.5, 'asd']) # AnyVector. Is this right? + self.assertEqual(so.metadata['list'], opentimelineio._otio.AnyVector([1, 2.5, 'asd'])) self.assertIsInstance(so.metadata['dict'], opentimelineio._otio.AnyDictionary) - # self.assertEqual(so.metadata['dict'], opentimelineio._otio.AnyDictionary({'map1': [345]})) + self.assertIsInstance(so.metadata['dict']['map1'], opentimelineio._otio.AnyVector) + self.assertEqual(so.metadata['dict'], opentimelineio._otio.AnyDictionary({'map1': [345]})) self.assertIsInstance(so.metadata['AnyVector'], opentimelineio._otio.AnyVector) - # self.assertEqual(list(so.metadata['AnyVector']), opentimelineio._otio.AnyVector([1, 'inside any vector'])) + self.assertEqual(list(so.metadata['AnyVector']), opentimelineio._otio.AnyVector([1, 'inside any vector'])) self.assertIsInstance(so.metadata['AnyDictionary'], opentimelineio._otio.AnyDictionary) - self.assertEqual(dict(so.metadata['AnyDictionary']), {'key_1': 1234, 'key_2': {'asdasdasd': 5.6}}) + self.assertEqual(so.metadata['AnyDictionary'], opentimelineio._otio.AnyDictionary({'key_1': 1234, 'key_2': {'asdasdasd': 5.6}})) self.assertEqual(so.metadata['RationalTime'], opentimelineio.opentime.RationalTime(value=10.0, rate=5.0)) self.assertEqual(so.metadata['TimeRange'], opentimelineio.opentime.TimeRange( opentimelineio.opentime.RationalTime(value=1.0), From 4949fc0654173a963a8226b1e047a54e8ee033df Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 4 Dec 2022 15:50:26 -0500 Subject: [PATCH 18/21] Adjust AnyVector python tests Signed-off-by: Jean-Christophe Morin --- tests/test_core_utils.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/test_core_utils.py b/tests/test_core_utils.py index a0a7b9425..b0755a61b 100644 --- a/tests/test_core_utils.py +++ b/tests/test_core_utils.py @@ -100,10 +100,10 @@ def test_main(self): v.append(2) self.assertEqual(len(v), 2) - self.assertEqual([value for value in v], [1, 2]) + self.assertEqual(v, [1, 2]) v.insert(0, 5) - self.assertEqual([value for value in v], [5, 1, 2]) + self.assertEqual(v, [5, 1, 2]) self.assertEqual(v[0], 5) self.assertEqual(v[-3], 5) @@ -124,13 +124,11 @@ def test_main(self): del v[0] self.assertEqual(len(v), 2) - # Doesn't work... - # assert v == [1, 100] - self.assertEqual([value for value in v], [1, 100]) + self.assertEqual(v, [1, 100]) del v[1000] # This will surprisingly delete the last item... self.assertEqual(len(v), 1) - self.assertEqual([value for value in v], [1]) + self.assertEqual(v, [1]) # Will delete the last item even if the index doesn't match. # It's a surprising behavior. @@ -144,7 +142,7 @@ def test_main(self): items.append(value) self.assertEqual(items, [1, '234', {}]) - self.assertFalse(v == [1, '234', {}]) # __eq__ is not implemented + self.assertTrue(v == [1, '234', {}]) self.assertTrue(1 in v) # Test __contains__ self.assertTrue('234' in v) @@ -181,13 +179,13 @@ def test_main(self): self.assertEqual(v3[1:7:2], [1, 3, 5]) del v3[2:7] - self.assertEqual(list(v3), [0, 1, 7, 8, 9]) + self.assertEqual(v3, [0, 1, 7, 8, 9]) v4 = opentimelineio.core._core_utils.AnyVector() v4.extend(range(10)) del v4[::2] - self.assertEqual(list(v4), [1, 3, 5, 7, 9]) + self.assertEqual(v4, [1, 3, 5, 7, 9]) v5 = opentimelineio.core._core_utils.AnyVector() tmplist = [1, 2] @@ -225,7 +223,7 @@ def test_raises_if_ref_destroyed(self): def test_copy(self): list1 = [1, 2, [3, 4], 5] copied = copy.copy(list1) - self.assertEqual(list(list1), list(copied)) + self.assertEqual(list1, copied) v = opentimelineio.core._core_utils.AnyVector() v.extend([1, 2, [3, 4], 5]) From c228768046023645b4123880d73b5758fa4a352f Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 4 Dec 2022 16:49:04 -0500 Subject: [PATCH 19/21] Lint Signed-off-by: Jean-Christophe Morin --- .../opentimelineio/core/__init__.py | 7 +- tests/test_serializable_object.py | 65 ++++++++++++------- 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio/core/__init__.py b/src/py-opentimelineio/opentimelineio/core/__init__.py index 7bc60d763..51a6dbf09 100644 --- a/src/py-opentimelineio/opentimelineio/core/__init__.py +++ b/src/py-opentimelineio/opentimelineio/core/__init__.py @@ -41,7 +41,12 @@ Metadata: TypeAlias = Union[Dict[str, 'MetadataValue'], AnyDictionary] """OTIO custom metadata type.""" -MetadataValue: TypeAlias = Union[bool, int, float, str, None, SerializableObject, RationalTime, TimeRange, TimeTransform, Metadata] +MetadataValue: TypeAlias = Union[ + bool, int, float, str, None, + SerializableObject, + RationalTime, TimeRange, TimeTransform, + Metadata +] """Metadata's values.""" from . _core_utils import ( # noqa diff --git a/tests/test_serializable_object.py b/tests/test_serializable_object.py index 5d0da426d..358e09540 100755 --- a/tests/test_serializable_object.py +++ b/tests/test_serializable_object.py @@ -4,9 +4,9 @@ # Copyright Contributors to the OpenTimelineIO project import opentimelineio as otio -import opentimelineio._otio +from opentimelineio._otio import AnyDictionary, AnyVector import opentimelineio.test_utils as otio_test_utils -import opentimelineio.opentime +from opentimelineio.opentime import RationalTime, TimeRange, TimeTransform import unittest import json @@ -39,11 +39,11 @@ def test_cons(self): self.assertEqual(so.metadata['foo'], 'bar') def test_cons2(self): - v = opentimelineio._otio.AnyVector() + v = AnyVector() v.append(1) v.append('inside any vector') - d = opentimelineio._otio.AnyDictionary() + d = AnyDictionary() d['key_1'] = 1234 d['key_2'] = {'asdasdasd': 5.6} so = otio.core.SerializableObjectWithMetadata( @@ -54,13 +54,16 @@ def test_cons2(self): 'dict': {'map1': [345]}, 'AnyVector': v, 'AnyDictionary': d, - 'RationalTime': opentimelineio.opentime.RationalTime(value=10.0, rate=5.0), - 'TimeRange': opentimelineio.opentime.TimeRange( - opentimelineio.opentime.RationalTime(value=1.0), - opentimelineio.opentime.RationalTime(value=100.0) + 'RationalTime': RationalTime( + value=10.0, + rate=5.0 ), - 'TimeTransform': opentimelineio.opentime.TimeTransform( - offset=opentimelineio.opentime.RationalTime(value=55.0), + 'TimeRange': TimeRange( + RationalTime(value=1.0), + RationalTime(value=100.0) + ), + 'TimeTransform': TimeTransform( + offset=RationalTime(value=55.0), scale=999 ) } @@ -69,22 +72,34 @@ def test_cons2(self): self.assertEqual(so.metadata['foo'], 'bar') self.assertEqual(so.metadata['string'], 'myvalue') self.assertEqual(so.metadata['int'], -999999999999) - self.assertIsInstance(so.metadata['list'], opentimelineio._otio.AnyVector) - self.assertEqual(so.metadata['list'], opentimelineio._otio.AnyVector([1, 2.5, 'asd'])) - self.assertIsInstance(so.metadata['dict'], opentimelineio._otio.AnyDictionary) - self.assertIsInstance(so.metadata['dict']['map1'], opentimelineio._otio.AnyVector) - self.assertEqual(so.metadata['dict'], opentimelineio._otio.AnyDictionary({'map1': [345]})) - self.assertIsInstance(so.metadata['AnyVector'], opentimelineio._otio.AnyVector) - self.assertEqual(list(so.metadata['AnyVector']), opentimelineio._otio.AnyVector([1, 'inside any vector'])) - self.assertIsInstance(so.metadata['AnyDictionary'], opentimelineio._otio.AnyDictionary) - self.assertEqual(so.metadata['AnyDictionary'], opentimelineio._otio.AnyDictionary({'key_1': 1234, 'key_2': {'asdasdasd': 5.6}})) - self.assertEqual(so.metadata['RationalTime'], opentimelineio.opentime.RationalTime(value=10.0, rate=5.0)) - self.assertEqual(so.metadata['TimeRange'], opentimelineio.opentime.TimeRange( - opentimelineio.opentime.RationalTime(value=1.0), - opentimelineio.opentime.RationalTime(value=100.0) + self.assertIsInstance(so.metadata['list'], AnyVector) + self.assertEqual( + so.metadata['list'], + AnyVector([1, 2.5, 'asd']) + ) + self.assertIsInstance(so.metadata['dict'], AnyDictionary) + self.assertIsInstance(so.metadata['dict']['map1'], AnyVector) + self.assertEqual(so.metadata['dict'], AnyDictionary({'map1': [345]})) + self.assertIsInstance(so.metadata['AnyVector'], AnyVector) + self.assertEqual( + so.metadata['AnyVector'], + AnyVector([1, 'inside any vector']) + ) + self.assertIsInstance(so.metadata['AnyDictionary'], AnyDictionary) + self.assertEqual( + so.metadata['AnyDictionary'], + AnyDictionary({'key_1': 1234, 'key_2': {'asdasdasd': 5.6}}) + ) + self.assertEqual( + so.metadata['RationalTime'], + RationalTime(value=10.0, rate=5.0) + ) + self.assertEqual(so.metadata['TimeRange'], TimeRange( + RationalTime(value=1.0), + RationalTime(value=100.0) )) - self.assertEqual(so.metadata['TimeTransform'], opentimelineio.opentime.TimeTransform( - offset=opentimelineio.opentime.RationalTime(value=55.0), + self.assertEqual(so.metadata['TimeTransform'], TimeTransform( + offset=RationalTime(value=55.0), scale=999 )) From cc60078652fd84e35e948ad503784a34b420c152 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sun, 4 Dec 2022 16:49:13 -0500 Subject: [PATCH 20/21] Remove py_to_any_dictionary Signed-off-by: Jean-Christophe Morin --- .../opentimelineio-bindings/otio_bindings.cpp | 5 ++--- .../opentimelineio-bindings/otio_utils.cpp | 8 -------- .../opentimelineio-bindings/otio_utils.h | 2 -- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_bindings.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_bindings.cpp index 74572125c..ce9a78baf 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_bindings.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_bindings.cpp @@ -157,10 +157,9 @@ static void set_type_record(SerializableObject* so, std::string schema_name) { } static SerializableObject* instance_from_schema(std::string schema_name, - int schema_version, py::object data) { - AnyDictionary object_data = py_to_any_dictionary(data); + int schema_version, AnyDictionaryProxy* data) { auto result = TypeRegistry::instance().instance_from_schema(schema_name, schema_version, - object_data, ErrorStatusHandler()); + data->fetch_any_dictionary(), ErrorStatusHandler()); return result; } diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index 74d4d0ce3..a2a8b8501 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -212,14 +212,6 @@ T py_to_cpp(py::handle const& o) { return o.cast(); } -AnyDictionary py_to_any_dictionary(py::object const& o) { - if (o.is_none()) { - return AnyDictionary(); - } - - return safely_cast_any_dictionary_any(py_to_any(o)); -} - py::object any_to_py(any const& a, bool top_level) { std::type_info const& tInfo = a.type(); auto e = _py_cast_dispatch_table.find(&tInfo); diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h index 1229d6d53..417a7b74b 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.h @@ -164,5 +164,3 @@ AnyDictionary py_to_cpp(pybind11::dict const& o); AnyVector py_to_cpp(pybind11::iterable const& o); template T py_to_cpp(pybind11::handle const& o); - -AnyDictionary py_to_any_dictionary(pybind11::object const& o); From 6086507901f8fe8f34c262929570370f290bce2e Mon Sep 17 00:00:00 2001 From: Jean-Christophe Morin Date: Sat, 8 Apr 2023 16:55:22 -0400 Subject: [PATCH 21/21] Add support for SerializableObjects in py_to_any Signed-off-by: Jean-Christophe Morin --- .../opentimelineio-bindings/otio_utils.cpp | 5 ++++ tests/test_core_utils.py | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp index a2a8b8501..e0c8b866c 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_utils.cpp @@ -162,6 +162,11 @@ any py_to_any(py::handle const& o) { return any(py_to_cpp(o)); } + if (py::isinstance(o)) { + SerializableObject::Retainer<> r(py::cast(o)); + return create_safely_typed_any(r.take_value()); + } + py::type pytype = py::type::of(o); throw py::type_error("Unsupported value type: " + py::cast(pytype.attr("__name__"))); } diff --git a/tests/test_core_utils.py b/tests/test_core_utils.py index b0755a61b..770618f61 100644 --- a/tests/test_core_utils.py +++ b/tests/test_core_utils.py @@ -2,11 +2,34 @@ import unittest import opentimelineio._otio +import opentimelineio.opentime import opentimelineio.core._core_utils class AnyDictionaryTests(unittest.TestCase): def test_main(self): + opentimelineio.core._core_utils.AnyDictionary({ + 'string': 'myvalue', + 'int': -999999999999, + 'list': [1, 2.5, 'asd'], + 'dict': {'map1': [345]}, + 'AnyVector': opentimelineio.core._core_utils.AnyVector(), + 'AnyDictionary': opentimelineio.core._core_utils.AnyDictionary(), + 'RationalTime': opentimelineio.opentime.RationalTime( + value=10.0, + rate=5.0 + ), + 'TimeRange': opentimelineio.opentime.TimeRange( + opentimelineio.opentime.RationalTime(value=1.0), + opentimelineio.opentime.RationalTime(value=100.0) + ), + 'TimeTransform': opentimelineio.opentime.TimeTransform( + offset=opentimelineio.opentime.RationalTime(value=55.0), + scale=999 + ), + 'SerializableObjectWithMetadata': opentimelineio._otio.SerializableObjectWithMetadata(), + }) + d = opentimelineio.core._core_utils.AnyDictionary() d['a'] = 1