Deprecate each_child() and each_clip() Python functions

[darbyjohnston]

The deprecated functions are in composition.py, serializable_collection.py, stack.py, timeline.py, and track.py.

Calls to these functions can be replaced with the new "children_if()" and "clip_if()" functions.

[meshula]

Synopsis

The names each_child and each_clip imply a Python iterator. These functions however return collections, not iterators, so they should be renamed

Proposal

rename each_child and each_clip to an appropriate new name.

Notes

child_if is not a Pythonic naming

all_children implies property access

[darbyjohnston]

@meshula When you say that "child_if" is not a Pythonic naming, is there documentation or a style guide that provides guidance on what is and is not proper naming?

@JeanChristopheMorinPerso Following up from our discussion in the PR, if the "all_children" implies a Python property, is there a variation that reads more like a function? Maybe "get_all_children"?

[jminor]

I like get_all_children(). I think that is clear, easy to remember, and makes sense when read aloud (my personal rule-of-thumb).

Python's standard libraries are somewhat inconsistent in naming conventions, so it is hard to point to a specific rule to follow. The Style Guide for Python doesn't say much on this topic except for this, which aligns with @JeanChristopheMorinPerso 's comment:

Note 2: Avoid using properties for computationally expensive operations; the attribute notation makes the caller believe that access is (relatively) cheap.

For code which does use properties, both the setter and getter tend to have the same name, without any get_ or set_ prefix, but in this case, using the get_ prefix can signal that there is some cost to calling the function, and can make it clear that there isn't a corresponding setter.

Ideally the doc string will clarify that this function gets all of the children recursively whereas the children property is shallow, and allows both get and set operations.

[darbyjohnston]

In that case maybe also get_children_if() and get_clips_if()? Or if the get_ prefix is sufficient we could drop the _if suffix and just have get_children() and get_clips(). The presumption being that children is the lightweight property and get_children() is a feature rich function with options for recursion and searching within a range.

How do the doc strings work with the Python wrappings?

[JeanChristopheMorinPerso]

I like get_all_children and get_all_clips I think, but I'm far from a good API designer. As for how docstrings are generated, see https://pybind11.readthedocs.io/en/stable/advanced/misc.html#generating-documentation-using-sphinx. Some examples: https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/main/src/py-opentimelineio/opentime-bindings/opentime_rationalTime.cpp#L108-L117.

[darbyjohnston]

Thanks for the link to opentime_rationalTime.cpp, I hadn't noticed those strings before.

Actually how about find_children() and find_clips() to replace children_if() and clip_if()? It doesn't sound like an attribute since the find_ implies there is some cost involved.

[meshula]

find also implies an operation that might involve rooting around a bit, which is accurate :)

[darbyjohnston]

My thought as well. :) I think it also implies find_ is configurable so it fits nicely with the search_range and shallow_search parameters, and possibly additional parameters in the future. Also that find_ could return all, some, or none of the children, in which case maybe the all_children() function is redundant. It also works with both the C++ and Python API.

[JeanChristopheMorinPerso]

I think I prefer find over get_all too!

[jminor]

Yes, find_ is a great choice here. 👍

[darbyjohnston]

Great, I updated the PR to use find_children and find_clips which seems to read nicer than children_if and clip_if. I think it also removes the need for the all_ functions, but check out the changes and see what you think.