Skip to content
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

remove_parents in feature detection only considers overlapping features detected at the previous threshold #298

Closed
w-k-jones opened this issue Jun 9, 2023 · 2 comments
Assignees
Labels
bug Code that is failing or producing the wrong result
Milestone

Comments

@w-k-jones
Copy link
Member

The remove_parents function only compares features with those at the previous threshold value. The assumption that any feature detected at a certain threshold must also be true with a fixed n_min_threshold across all thresholds, but can be false if a range of n_min_threshold is provided (see e.g. the final notebook example in #283)

We should modify the way we store threshold regions in feature detection so that all previously detected features are considered by remove_parents, not just those at the previous threshold.

@w-k-jones w-k-jones added the bug Code that is failing or producing the wrong result label Jun 9, 2023
@w-k-jones w-k-jones added this to the Version 1.5 milestone Jun 9, 2023
@w-k-jones w-k-jones self-assigned this Jun 9, 2023
@freemansw1
Copy link
Member

Thanks for this report; I don't think we considered this downstream consequence of #283 and the change to n_min_threshold.

We should probably hold off merging #283 until this is resolved.

@freemansw1
Copy link
Member

Given that this has been resolved in #283, I'm going to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code that is failing or producing the wrong result
Projects
None yet
Development

No branches or pull requests

2 participants