-
Notifications
You must be signed in to change notification settings - Fork 293
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
Fix for find_children() with stacks #2 #1766
Fix for find_children() with stacks #2 #1766
Conversation
Signed-off-by: Darby Johnston <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1766 +/- ##
==========================================
- Coverage 84.11% 81.69% -2.43%
==========================================
Files 198 176 -22
Lines 22241 12342 -9899
Branches 4687 3027 -1660
==========================================
- Hits 18709 10083 -8626
+ Misses 2610 1723 -887
+ Partials 922 536 -386
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 66 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
@jminor Sorry, I managed to mess up the git history on the previous PR while trying to update with the latest changes from main. I created this new PR as a replacement. I believe your last comment was about using |
There's an example of a track with a source_range in Figure 3 here: There's also an example of a stack with a source_range in Figure 4 on that same page. In that case the source_range is used to trim the nested stack. Although rare, the source_range could be set on the top level stack of a timeline also. |
Signed-off-by: Darby Johnston <[email protected]>
Thanks, those figures were very helpful; I guess I should have checked there first. :) I changed the code to use |
@jminor Just checking if you have time to review the latest changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. The use of trimmed_range_in_parent looks correct to me :)
Thanks! |
Fixes #1652
This PR is a replacement for #1755 which has some trouble with the git history.
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.