Issue on OSX with AnyDictionary object

[Tilix4]

Bug Report

Incorrect Functionality and General Questions

OTIO python lib doesn't behave the same way on windows and mac, regarding dictionaries, which may lead to bugs.

To Reproduce

⚠️ This is a tricky one

This is an inconsistent behaviour between OSX and Windows.
I spotted the issue while using OTIO with Blender, and I have no idea about how to reproduce it either way, then it includes other dependencies, sorry for that.

Reproduce (have to be executed on Mac and on Windows, not tested on Linux)

1. Download Blender 2.92
2. Install OTIO into Blender: you can follow these instructions. Make sure you install opentimelineio==0.11.0 on windows, else it won't work due to Windows: Import error after pip install #813.
3. Start Blender with a console.
4. Open this blend file (after unzipping): otio_dict_bug.blend.zip
5. Run the script in the top left corner
6. Look at the console: Results aren't the same in Blender depending on the platform.

That's because on Windows, loaded from file dictionaries are dict and on Mac they are opentimelineio._otio.AnyDictionary, which makes Blender not recognize them.

Expected Behavior

Dictionaries must be strictly identical across the platforms, then be dict on Mac.

Screenshots

Mac:

Win:

[meshula]

Thanks for the report. Pybind11 is wrapping any; we've just updated the version of pybind11 OTIO uses; we need to check if the latest pybind11 has an issue similar to what we just resolved for optional (#902).

[ssteinbach]

Couple of thoughts on this:

1. if you're installing with OTIO 11.0, thats the last pure python version. So you shouldn't be seeing AnyDictionaries at all, they should all be normal python dictionaries.
2. If you're comparing the type of the loaded dictionaries from a C++OTIO (say, 0.13) to one from a pure python OTIO (0.11), the _type_ of the dictionary will be different. But the content of the dictionary should be the same.

I would consider the fact that the Metadata dictionary is of type dictionary (or AnyDictionary) to be an implementation detail. They both subscribe to the python MutableMapping API, so following duck typing rules they should be the same. (for reference: https://docs.python.org/3/library/collections.abc.html#collections.abc.MutableMapping)

In short, I'm not sure that this is a bug, but if there is some kind of behavior you're trying to build around the underlying types of the dictionaries, I'm sure we can find another way to help you get what you need.

I'm going to close this for now, feel free to re-open or ping us on slack if you'd like to discuss.

[ssteinbach]

apologies in advance for the long answer

Why is the .metadata dictionary no longer a python dict

A follow up question came up on slack:

I don't contest the viability of using MutableMapping API, I'm only facing a case where a code expects a type == dict where not using this expected python type leads to wrong interpretation by Blender. Also, I don't understand why we don't get a dict if it's actually supposed to be a dict . Even if it looks like a dict, acts like a dict and walks like a dict

To explain what I meant, there are three major points:

1. It isn't a dict, in fact, it can't be of type dict because of the nature of python-wrapped C++
2. Even if it was possible (as was the case in the pure python implementation of OTIO), it probably shouldn't have been dict, because of other constraints
3. When I encounter code that checks type(thing) == dict (or type(thing) == list) in python code reviews, I discourage it because it typically falls down in cases like this

1: The metadata container cannot be of type dict

In the original, pure python, implementation of OTIO, the metadata dictionaries were stored in builtin python dictionaries:

https://github.com/PixarAnimationStudios/OpenTimelineIO/blob/cc78a7c5e9b10808d524edd00a09f8a11b21ce1a/opentimelineio/core/composable.py#L65

Truthfully, we were already talking about moving off of dict to a mutable-mapping derived type because of reason I'll talk about in 2, even in a pure python world. That said, when we built out the C++ core, we didn't have a choice.

The C++ core is intended to still be useful even if the python SDK/interpreter is not present. This is so that it can be linked into applications that don't use python and do not want it as a dependency.

To implement the metadata dictionaries in C++, we used the any library, which bridges an any-like type from C++17 back to C++11. This then gets wrapped up in the AnyDictionary class we implement:

https://github.com/PixarAnimationStudios/OpenTimelineIO/blob/main/src/opentimelineio/anyDictionary.h

Finally, to expose the C++ library to python users, we wrap it using the pybind11 library. The reason it isn't a python dictionary is that under the hood, it simply isn't -- those interfaces are manipulating the underlying C++ AnyDictionary objects in memory without being copied into pure python objects.

2: It probably shouldn't have been a dict anyway

Even prior to #892, we knew that we would probably have to be more specific in what kinds of types and objects we serialize to JSON. It was possible to put things in dictionaries, and then attempt to serialize and get an error. This is not an ideal user experience -- better would be an error when adding the non-serializable object object to the metadata dictionary in the first place.

To do that we would have probably added a MutableMapping derived class that would do the extra error checking on insertion.

In the C++ implementation, we are specific about which types we allow in metadata dictionaries, Any types and serialization:

https://opentimelineio.readthedocs.io/en/latest/tutorials/otio-file-format-specification.html#number-types

3: checking for either isinstance(dict) or type == dict is probably not ideal anyway

The first issue is that checking type == dict prevents anyone from building subclasses that meet the same interface from matching this condition. The second issue is that python doesn't want you to derive from dict when subclassing, they want you to subclass from the collections.abc classes.

You can read about that here:
https://treyhunner.com/2019/04/why-you-shouldnt-inherit-from-list-and-dict-in-python/#Making_a_custom_dictionary

Or in the python documentation, under dictionary, it also points at the collections.abc.Mapping class:
https://docs.python.org/3/reference/datamodel.html#basic-customization

Instead, they want you to inherit from collections.abc.Mapping when building dictionary like-classes. You'll notice that our wrapped AnyDictionary also satisfies this constraint because of pybind11.

>>> import opentimelineio as otio
>>> cl = otio.schema.Clip()
>>> cl.metadata
{}
>>> type(cl.metadata)
<class 'opentimelineio._otio.AnyDictionary'>
>>> isinstance(cl.metadata, dict)
False
>>> import collections
>>> isinstance(cl.metadata, collections.abc.Mapping)
True