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

Fix FromIterator for Segtree #138

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mousecrusher2
Copy link

This pull request fixes the FromIterator implementation for the Segtree struct. The previous implementation use Iterator::size_hint().0 to get length of the iterator. However, that is incorrect.

@TonalidadeHidrica TonalidadeHidrica self-requested a review February 13, 2024 14:05
@TonalidadeHidrica
Copy link
Collaborator

TonalidadeHidrica commented Mar 25, 2024

You are correct. The current implementation, introduced by @mizar in #115, uses size_hint().0 as the number of elements in the segtree, but this is actually the lower bound of the element that is actually yielded from the iterator; moreover, it may be completely wrong. It's my fault overlooking it, thank you for pointing it out.

I guess however that my implementation in #107 is still correct. The implementation uses size_hint().0 to avoid reallocation when the number of elements is estimated correctly. It also fix the length of the slice for the parent nodes if there are not enough or too many of them compared to the number actually obtained by the iterator. Thus, I think this PR is superceded by #107, provided that I add the filter test that you suggested.

(By the way, despite #115 (comment), the FromIterator was not actually reverted. That's also my oversight...)

Comment me if someone (the OP or a maintainer) has any concern; otherwise I'm closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants