search

AcademySoftwareFoundation / OpenTimelineIO

Open Source API and interchange format for editorial timeline information.
http://opentimeline.io
Apache License 2.0
1.45k stars 287 forks source link

Fix for find_children() with stacks #1755

Closed darbyjohnston closed 3 months ago

darbyjohnston commented 4 months ago

Fixes #1652

After some investigation, it looks like the find_children() function was not working with stacks when given a search range. To fix this, an override is added for the function children_in_range() that works with "stacks" of children.

darbyjohnston commented 3 months ago

Thanks, I think the bug did predate the C++ port since the confidence tests didn't cover it. That's an interesting point about the optimization, If the bisect code was for Python it would be interesting to test how much it helps in C++.

jminor commented 3 months ago

Oh, I thought of one other edge case... Could you try this with a Track that has a source_range trim on it? I suspect that the new function you made might need to use trimmed_range_in_parent instead of trimmed_range.

darbyjohnston commented 3 months ago

I didn't realize tracks could have a source range, how does that affect the clips? If the track source range duration is shorter than the clips, are they trimmed?

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.68%. Comparing base (c0e97b0) to head (d8ab747). Report is 11 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755/graphs/tree.svg?width=650&height=150&src=pr&token=mfnfUB4h7u&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation)](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ```diff @@ Coverage Diff @@ ## main #1755 +/- ## ========================================== - Coverage 84.11% 81.68% -2.44% ========================================== Files 198 176 -22 Lines 22241 12342 -9899 Branches 4687 3023 -1664 ========================================== - Hits 18709 10082 -8627 + Misses 2610 1718 -892 + Partials 922 542 -380 ``` | [Flag](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [py-unittests](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | `81.68% <88.23%> (-2.44%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | Coverage Δ | | |---|---|---| | [src/opentimelineio/composition.h](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=src%2Fopentimelineio%2Fcomposition.h&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-c3JjL29wZW50aW1lbGluZWlvL2NvbXBvc2l0aW9uLmg=) | `65.51% <ø> (ø)` | | | [src/opentimelineio/stackAlgorithm.cpp](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=src%2Fopentimelineio%2FstackAlgorithm.cpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-c3JjL29wZW50aW1lbGluZWlvL3N0YWNrQWxnb3JpdGhtLmNwcA==) | `56.70% <100.00%> (ø)` | | | [...opentimelineio/opentimelineio/adapters/\_\_init\_\_.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=src%2Fpy-opentimelineio%2Fopentimelineio%2Fadapters%2F__init__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-c3JjL3B5LW9wZW50aW1lbGluZWlvL29wZW50aW1lbGluZWlvL2FkYXB0ZXJzL19faW5pdF9fLnB5) | `85.18% <100.00%> (-2.32%)` | :arrow_down: | | [...timelineio/console/autogen\_plugin\_documentation.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=src%2Fpy-opentimelineio%2Fopentimelineio%2Fconsole%2Fautogen_plugin_documentation.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-c3JjL3B5LW9wZW50aW1lbGluZWlvL29wZW50aW1lbGluZWlvL2NvbnNvbGUvYXV0b2dlbl9wbHVnaW5fZG9jdW1lbnRhdGlvbi5weQ==) | `64.10% <ø> (-7.94%)` | :arrow_down: | | [...-opentimelineio/opentimelineio/plugins/manifest.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=src%2Fpy-opentimelineio%2Fopentimelineio%2Fplugins%2Fmanifest.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-c3JjL3B5LW9wZW50aW1lbGluZWlvL29wZW50aW1lbGluZWlvL3BsdWdpbnMvbWFuaWZlc3QucHk=) | `87.50% <ø> (+2.88%)` | :arrow_up: | | [tests/test\_builtin\_adapters.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=tests%2Ftest_builtin_adapters.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-dGVzdHMvdGVzdF9idWlsdGluX2FkYXB0ZXJzLnB5) | `96.29% <ø> (+0.46%)` | :arrow_up: | | [tests/test\_console.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=tests%2Ftest_console.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-dGVzdHMvdGVzdF9jb25zb2xlLnB5) | `95.31% <ø> (-0.02%)` | :arrow_down: | | [tests/test\_examples.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=tests%2Ftest_examples.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-dGVzdHMvdGVzdF9leGFtcGxlcy5weQ==) | `90.00% <ø> (-0.48%)` | :arrow_down: | | [tests/test\_otiod.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=tests%2Ftest_otiod.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-dGVzdHMvdGVzdF9vdGlvZC5weQ==) | `96.87% <ø> (-0.05%)` | :arrow_down: | | [tests/test\_otioz.py](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree&filepath=tests%2Ftest_otioz.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation#diff-dGVzdHMvdGVzdF9vdGlvei5weQ==) | `97.93% <ø> (-0.03%)` | :arrow_down: | | ... and [4 more](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) | | ... and [57 files with indirect coverage changes](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Last update [4b3b673...d8ab747](https://app.codecov.io/gh/AcademySoftwareFoundation/OpenTimelineIO/pull/1755?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AcademySoftwareFoundation).
darbyjohnston commented 3 months ago

Replaced by #1766.