Fix for SerializableCollection::children_if

[darbyjohnston]

Signed-off-by: Darby Johnston [email protected]

Fixes #1400

Fixes an issue where SerializableCollection::children_if doesn't recurse into timelines. I'm marking this as a draft for now since it doesn't address all of the items in the issue (Python tests and missing arguments).

[codecov-commenter]

Codecov Report

Merging #1404 (5c1f721) into main (cbef407) will increase coverage by 0.07%.
The diff coverage is 96.62%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1404      +/-   ##
==========================================
+ Coverage   85.99%   86.06%   +0.07%     
==========================================
  Files         200      201       +1     
  Lines       20934    21062     +128     
  Branches     2459     2461       +2     
==========================================
+ Hits        18002    18127     +125     
- Misses       2331     2333       +2     
- Partials      601      602       +1     

Flag	Coverage Δ
py-unittests	86.06% <96.62%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...c/py-opentimelineio/opentimelineio/schema/stack.py	66.66% <50.00%> (ø)
src/opentimelineio/serializableCollection.h	85.71% <80.00%> (-1.79%)	⬇️
...entimelineio-bindings/otio_serializableObjects.cpp	91.57% <88.88%> (ø)
tests/test_track.py	95.34% <95.34%> (ø)
...o/opentimelineio/schema/serializable_collection.py	100.00% <100.00%> (ø)
...y-opentimelineio/opentimelineio/schema/timeline.py	100.00% <100.00%> (ø)
...c/py-opentimelineio/opentimelineio/schema/track.py	100.00% <100.00%> (ø)
tests/test_serializable_collection.py	97.40% <100.00%> (+3.28%)	⬆️
tests/test_timeline.py	94.84% <100.00%> (+0.97%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbef407...5c1f721. Read the comment docs.

[darbyjohnston]

I think I have addressed all the items from the issue as well as adding more consistent use of the "shallow_search" parameter to the various functions. @rogernelson has some interesting thoughts on a more efficient implementation of the "children_if" function, but maybe we need to decide whether the recursive searching functionality is actually appropriate for SerializableCollection.

[ssteinbach]

I think we should probably not do a major functionality switch on SerializableCollection right now, since we're in a release process. That is a conversation we can have for beta 16. Lets review this PR with its current functionality, and circle back to that other discussion later.

Thanks @darbyjohnston! I'll take a look at the code.

[ssteinbach]

Looks good. Saw (maybe) a copy paste typo in the unit tests? I probably missed some but I might just be misunderstanding the use of assertTrue, too. Let me know what you think!

[ssteinbach]

A few more assertTrue still in there

[ssteinbach]

Alright, this looks great. Thanks @darbyjohnston