Remove deprecated each_child and each_clip functions

[darbyjohnston]

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

Fixes #1409

This PR removes the deprecated Python functions "each_child" and "each_clip", replacing them with "children_if" and "clip_if".

Python is not my expertise so here are a couple of notes about the changes I wasn't sure about:

• stack.py and track.py effectively became empty files after removing the functions. The linter didn't like Python files with just imports so I removed them.
• In a previous PR I had removed the function "each_clip" in clip.py which caused some issues and was later reverted. This time I renamed it to "clip_if", is that necessary?
• As far as I understand the "each_child" and "each_clip" functions were iterators? The new functions return lists, so the couple places that were calling "next" I replaced with a list index.

[reinecke]

From TSC meeting: Should we consider adding something like all_children and all_clips for readability in the cases where there are no predicates?

[JeanChristopheMorinPerso]

Maybe get_children and get_clips to make it obvious that they are not properties?

[codecov-commenter]

Codecov Report

Merging #1437 (28d1ab9) into main (33e0d1a) will decrease coverage by 0.01%.
The diff coverage is 84.66%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1437      +/-   ##
==========================================
- Coverage   80.09%   80.07%   -0.02%     
==========================================
  Files         198      196       -2     
  Lines       21703    21677      -26     
  Branches     4298     4298              
==========================================
- Hits        17382    17357      -25     
+ Misses       2174     2173       -1     
  Partials     2147     2147              

Flag	Coverage Δ
py-unittests	80.07% <84.66%> (-0.02%)	⬇️

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

Impacted Files	Coverage Δ
src/opentimelineio/track.h	100.00% <ø> (ø)
...timelineio/opentimelineio/algorithms/stack_algo.py	64.28% <0.00%> (ø)
...-opentimelineio/opentimelineio/core/composition.py	100.00% <ø> (ø)
...y-opentimelineio/opentimelineio/schema/__init__.py	100.00% <ø> (ø)
...o/opentimelineio/schema/serializable_collection.py	100.00% <ø> (ø)
...y-opentimelineio/opentimelineio/schema/timeline.py	100.00% <ø> (ø)
tests/test_item.py	98.66% <ø> (ø)
src/opentimelineio/stack.cpp	57.35% <33.33%> (ø)
...entimelineio-bindings/otio_serializableObjects.cpp	36.09% <44.82%> (ø)
src/opentimelineio/composition.h	41.37% <50.00%> (ø)
... and 27 more

---

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 33e0d1a...28d1ab9. Read the comment docs.

[darbyjohnston]

I added the "all_children()" and "all_clips" functions, check out the changes and see if they look reasonable. I think in the meeting we had talked about them being Python functions, but I thought if they are useful why not add them to the C++ API as well? It might also be worth considering adding convenience functions for getting the first child or clip which can be useful.

I'm not sure why the "deps" directory is showing up in this PR, I think it got added when I merged the latest code from main.

[darbyjohnston]

I think I fixed the "deps" commits but I'm not sure why they changed when I merged the main branch. Just for reference here are the steps I used:

Show the differences:

$ git diff main..remove_each_child -- src/deps
diff --git a/src/deps/optional-lite b/src/deps/optional-lite
...
-Subproject commit be720ebfd7add22abe60e49602735de9231105d0
+Subproject commit 7a66721b8da0affc30e43f9272a8635259562048
diff --git a/src/deps/pybind11 b/src/deps/pybind11
...
-Subproject commit 5bc0943ed96836f46489f53961f6c438d2935357
+Subproject commit 0176632e8cef72a4223f2df54d6dfb9e552ec71f

Change to the submodule directories and check out the previous commits:

$ cd src/deps/optional-lite
$ git checkout be720ebfd7add22abe60e49602735de9231105d0
$ cd ../pybind11
$ git checkout 5bc0943ed96836f46489f53961f6c438d2935357

Check in the changes:

cd ../../..
git add src/deps
git commit -m "Revert submodule" --signoff

[meshula]

Does this PR need a rebase?

[darbyjohnston]

I'm not sure, does it?

Also the current build failure is from an error uploading the code coverage:

['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 404 - {'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}

I guess a temporary thing, but I didn't see a way to manually re-run the tests.

[meshula]

Ok, no it doesn't need a rebase. When I checked last, the diff also showed all the CI modifications for Python 3.11.

[meshula]

I re-ran all the checks just now.

[meshula]

codecov failed straight up again. @JeanChristopheMorinPerso Do you know what the issue is?

[JeanChristopheMorinPerso]

I unfortunately have no idea why it failed. Maybe just a glitch in the matrix... I'm guessing a retry might work.

[darbyjohnston]

I think it may be an intermittent issue with CodeCov, I had a similar failure in one of my projects recently.

[darbyjohnston]

@JeanChristopheMorinPerso You had suggested some alternate names:

Maybe get_children and get_clips to make it obvious that they are not properties?

I'm not very familiar with Python, what is the issue with properties?

[JeanChristopheMorinPerso]

I'm not very familiar with Python, what is the issue with properties?

Oh I missed your message. Nothing wrong with properties. My comment was more about the fact that all_children sounds like it is a property while it is actually a method. Python being Python, it's quite easy to think that something is a property and not call it like a method/function and it will still work but won't do what we think it would do.

[meshula]

I'd like to suggest we take the naming debate back to the issue.

#1409

[meshula]

The form of the code is fine, let's agree on the naming before proceeding.

[jminor]

This looks great! I just had two questions/comments in-line.

[jminor]

I think this is ready to go. @meshula can you take a last look? I see GitHub says you requested a change.

Thanks for persevering through our indecision with this @darbyjohnston - I think the find_ naming really works great!

[meshula]

looks good to me! thanks