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

Add to_dict methods #1705

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 60 additions & 0 deletions src/py-opentimelineio/opentimelineio/core/_core_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import types
import collections.abc
import copy
import json

from .. import (
_otio,
Expand All @@ -14,6 +15,11 @@
AnyVector,
PyAny
)
from .. _opentime import (
RationalTime,
TimeRange,
TimeTransform
)


SUPPORTED_VALUE_TYPES = (
Expand Down Expand Up @@ -388,3 +394,57 @@ def __deepcopy__(self, *args, **kwargs):
@add_method(SerializableObject)
def __copy__(self, *args, **kwargs):
raise ValueError("SerializableObjects may not be shallow copied.")


@add_method(AnyDictionary) # noqa: F811
def to_dict(self): # noqa: F811
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not convinced that this is the best name. Should we instead call it to_py? It would be more generic. I don't like that we have to_dict on everything except AnyVector which has to_list.

Copy link

Choose a reason for hiding this comment

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

to_json?

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no type named json in Python. to_json to me means "to JSON formatted string". And it's not json specific, it's really just a pure python data structure.

Copy link

Choose a reason for hiding this comment

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

OK I see what you mean. to_py is indeed more generic then, even if it's a bit less intuitive?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to_dict and to_list over to_py.
That way you know what you get.
Of course the nature of the object will indicate what to_py returns, but I still prefer the explicitness of to_dict and to_list

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't mind using to_dict because I'm used to seeing and using this to make a complex structure a simple dictionary. To me it makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

to_py would allow us to iterate through a list of OTIO objects (of any type) and convert them to python types without using isinstance to pick the right method.

Copy link
Contributor

Choose a reason for hiding this comment

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

Follow up to this, I ended up having to make a helper function to do exactly this at work. I ended up calling the function pythonify to communicate its returning Python objects.

to_py seems in line with other conversion method names on OTIO objects and is clear of what to expect. Another reason to use a py/python name is because AnyDictionary and AnyVector can nest each other so you're not just getting a dict or list but rather the entire struct is pythonify-ed.

"""
Convert to a built-in dict. It will recursively convert all values
to their corresponding python built-in types.
"""
return json.loads(_otio._serialize_json_to_string(_value_to_any(self), {}, 0))


@add_method(AnyVector) # noqa: F811
def to_list(self): # noqa: F811
"""
Convert to a built-in list. It will recursively convert all values
to their corresponding python built-in types.
"""
return json.loads(_otio._serialize_json_to_string(_value_to_any(self), {}, 0))


@add_method(SerializableObject) # noqa: F811
def to_dict(self): # noqa: F811
"""
Convert to a built-in dict. It will recursively convert all values
to their corresponding python built-in types.
"""
return json.loads(_otio._serialize_json_to_string(_value_to_any(self), {}, 0))


@add_method(RationalTime) # noqa: F811
def to_dict(self): # noqa: F811
"""
Convert to a built-in dict. It will recursively convert all values
to their corresponding python built-in types.
"""
return json.loads(_otio._serialize_json_to_string(_value_to_any(self), {}, 0))


@add_method(TimeRange) # noqa: F811
def to_dict(self): # noqa: F811
"""
Convert to a built-in dict. It will recursively convert all values
to their corresponding python built-in types.
"""
return json.loads(_otio._serialize_json_to_string(_value_to_any(self), {}, 0))


@add_method(TimeTransform) # noqa: F811
def to_dict(self): # noqa: F811
"""
Convert to a built-in dict. It will recursively convert all values
to their corresponding python built-in types.
"""
return json.loads(_otio._serialize_json_to_string(_value_to_any(self), {}, 0))
52 changes: 52 additions & 0 deletions tests/test_core_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@
# Copyright Contributors to the OpenTimelineIO project

import copy
import json
import unittest

import opentimelineio._otio
import opentimelineio.core._core_utils
import opentimelineio.core
import opentimelineio.opentime


class AnyDictionaryTests(unittest.TestCase):
Expand Down Expand Up @@ -242,3 +245,52 @@ def test_copy(self):
deepcopied = copy.deepcopy(v)
self.assertIsNot(v, deepcopied)
self.assertIsNot(v[2], deepcopied[2])


class ConvertToPython(unittest.TestCase):
def test_SerializableObject(self):
so = opentimelineio.core.SerializableObjectWithMetadata(name="asd")
so.metadata["key1"] = opentimelineio.core.Composition()

converted = so.to_dict()
self.assertTrue(isinstance(converted, dict))
json.dumps(converted)

def test_AnyDictionary(self):
ad = opentimelineio._otio.AnyDictionary()
ad["my key"] = opentimelineio.core.Composable()

converted = ad.to_dict()
self.assertTrue(isinstance(converted, dict))
json.dumps(converted)

def test_AnyVector(self):
av = opentimelineio._otio.AnyVector()
av.append(1)
av.append(opentimelineio._otio.AnyDictionary())

converted = av.to_list()
self.assertTrue(isinstance(converted, list))
self.assertEqual(converted, [1, {}])
json.dumps(converted)

def test_RationalTime(self):
rt = opentimelineio.opentime.RationalTime()

converted = rt.to_dict()
self.assertTrue(isinstance(converted, dict))
json.dumps(converted)

def test_TimeRange(self):
tr = opentimelineio.opentime.TimeRange()

converted = tr.to_dict()
self.assertTrue(isinstance(converted, dict))
json.dumps(converted)

def test_TimeTransform(self):
tt = opentimelineio.opentime.TimeTransform()

converted = tt.to_dict()
self.assertTrue(isinstance(converted, dict))
json.dumps(converted)
Loading