Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Improve typing of the metadata parameter #1490

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1592989
First try at improving type of metadata
JeanChristopheMorinPerso Oct 5, 2022
7fb3aeb
debug
JeanChristopheMorinPerso Oct 5, 2022
8ae911b
tmp
JeanChristopheMorinPerso Oct 6, 2022
126e694
Progress! Things are working ish
JeanChristopheMorinPerso Oct 6, 2022
0866d9f
more types (crashing)
JeanChristopheMorinPerso Oct 6, 2022
9009096
Working state, but incomplete
JeanChristopheMorinPerso Oct 6, 2022
abd6504
Some notes about adding a custom caster for AnyDictionary
JeanChristopheMorinPerso Oct 6, 2022
40a41f0
Little tweak in naming
JeanChristopheMorinPerso Oct 15, 2022
acea32b
Fix bad rebase
JeanChristopheMorinPerso Nov 4, 2022
a9de0f2
Allow AnyDictionary creation from dict and AnyVector from sequence
JeanChristopheMorinPerso Nov 5, 2022
27b9265
Completey removed py_to_any. No more roundtrip between C++ and Python…
JeanChristopheMorinPerso Nov 6, 2022
cebb945
Clean init of AnyVectorProxy
JeanChristopheMorinPerso Nov 6, 2022
2a53b9d
Implement a working type caster that takes either an AnyDictionaryPro…
JeanChristopheMorinPerso Nov 6, 2022
f7fe99d
Make py_to_any raise type_error
JeanChristopheMorinPerso Nov 6, 2022
9c270af
Use AnyDictionaryProxy everywhere instead of py::object
JeanChristopheMorinPerso Nov 6, 2022
c9e05d6
Add support for RationalTime, TimeRange and TimeTransform
JeanChristopheMorinPerso Dec 4, 2022
dc6e4d5
Add support to check equality of AnyVector
JeanChristopheMorinPerso Dec 4, 2022
4949fc0
Adjust AnyVector python tests
JeanChristopheMorinPerso Dec 4, 2022
c228768
Lint
JeanChristopheMorinPerso Dec 4, 2022
cc60078
Remove py_to_any_dictionary
JeanChristopheMorinPerso Dec 4, 2022
6086507
Add support for SerializableObjects in py_to_any
JeanChristopheMorinPerso Apr 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---------------------------------------------------------------
Expand Down Expand Up @@ -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)
1 change: 1 addition & 0 deletions src/opentimelineio/serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>(value));
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ void otio_any_dictionary_bindings(py::module m) {

py::class_<AnyDictionaryProxy>(m, "AnyDictionary")
.def(py::init<>())
.def(py::init([](py::dict item) {
AnyDictionary d = py_to_cpp(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)
Expand Down
72 changes: 70 additions & 2 deletions src/py-opentimelineio/opentimelineio-bindings/otio_anyDictionary.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,29 @@
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;

AnyDictionary::iterator ptr;
for (ptr = other.any_dictionary->begin(); ptr != other.any_dictionary->end(); ptr++) {
d->insert(*ptr);
}

any_dictionary = d;
}

~AnyDictionaryProxy() {
}

using MutationStamp = AnyDictionary::MutationStamp;

static void throw_dictionary_was_deleted() {
throw py::value_error("Underlying C++ AnyDictionary has been destroyed");
Expand Down Expand Up @@ -99,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<AnyDictionaryProxy> : public type_caster_base<AnyDictionaryProxy> {
using base = type_caster_base<AnyDictionaryProxy>;
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<py::dict>(src)) {
auto proxy = new AnyDictionaryProxy();
AnyDictionary&& d = py_to_cpp(py::cast<py::dict>(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);
// }
};
}}
10 changes: 6 additions & 4 deletions src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ void otio_any_vector_bindings(py::module m) {

py::class_<AnyVectorProxy>(m, "AnyVector")
.def(py::init<>())
.def(py::init([](const py::iterable &it) {
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)
.def("__internal_delitem__", &AnyVectorProxy::del_item, "index"_a)
.def("__len__", &AnyVectorProxy::len)
.def("__internal_insert", &AnyVectorProxy::insert)
.def("__iter__", &AnyVectorProxy::iter, py::return_value_policy::reference_internal);
}




16 changes: 16 additions & 0 deletions src/py-opentimelineio/opentimelineio-bindings/otio_anyVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@ namespace py = pybind11;
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;

AnyVector::iterator ptr;
for (ptr = other.any_vector->begin(); ptr < other.any_vector->end(); ptr++) {
v->push_back(*ptr);
}
any_vector = v;
}

static void throw_array_was_deleted() {
throw py::value_error("Underlying C++ AnyVector object has been destroyed");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading