Skip to content

Conversation

@davidhewitt
Copy link
Member

Related to #5661

This PR removes counters for remaining values from set and frozenset iterators; the underlying Python iterators already have this information and we can query it when needed.

While touching the code, I replaced some .unwrap() calls to .expect() to improve self-documentation slightly.

@davidhewitt davidhewitt changed the title don't count remaining elements in set and frozenset iteration remove redundant counters for remaining elements in set and frozenset iteration Jan 9, 2026

fn size_hint(&self) -> (usize, Option<usize>) {
(self.remaining, Some(self.remaining))
let len = self.len();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to call self.__length_hint__() here? (Or whatever the Rust/C name of that is.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to call ExactSizeIterator::len to show where this value is coming from (that len does call size_hint of the underlying iterator, yes).

@davidhewitt davidhewitt enabled auto-merge January 11, 2026 09:50
@davidhewitt davidhewitt added this pull request to the merge queue Jan 11, 2026
Merged via the queue into PyO3:main with commit 0f4c7ca Jan 11, 2026
42 of 43 checks passed
@davidhewitt davidhewitt deleted the set-size-hint branch January 11, 2026 11:17
@Icxolu
Copy link
Contributor

Icxolu commented Jan 12, 2026

@davidhewitt Looks like there is some issue with this. On current main I get two FrozenSet test failures when using the abi3 feature:

❯ cargo test --package pyo3 --lib -F abi3 -- types::frozenset::tests --nocapture
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.19s
     Running unittests src\lib.rs (target\x86_64-pc-windows-gnullvm\debug\deps\pyo3-9eb94db30d3f0e35.exe)

running 8 tests
test types::frozenset::tests::test_frozenset_builder ... 
thread 'types::frozenset::tests::test_iter_count' (8760) panicked at src\types\frozenset.rs:348:13:
assertion `left == right` failed
  left: 0
 right: 3
oknote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

test types::frozenset::tests::test_iter_count ... FAILED
thread 'types::frozenset::tests::test_frozenset_iter_size_hint' (9804) panicked at src\types\frozenset.rs:315:13:
assertion `left == right` failed
  left: 0
 right: 1

test types::frozenset::tests::test_frozenset_iter ... ok
test types::frozenset::tests::test_frozenset_iter_bound ... ok
test types::frozenset::tests::test_frozenset_empty ... ok
test types::frozenset::tests::test_frozenset_iter_size_hint ... FAILED
test types::frozenset::tests::test_frozenset_contains ... ok
test types::frozenset::tests::test_frozenset_new_and_len ... ok

failures:

failures:
    types::frozenset::tests::test_frozenset_iter_size_hint
    types::frozenset::tests::test_iter_count

test result: FAILED. 6 passed; 2 failed; 0 ignored; 0 measured; 763 filtered out; finished in 0.03s

@Icxolu
Copy link
Contributor

Icxolu commented Jan 12, 2026

I think the problem is that Bound<'py, PyIterator> only overrides Iterator::size_hint on the unlimited API. On the limited api the default from Iterator is used, which returns (0, None). Not sure why CI did not catch that tho...

@davidhewitt
Copy link
Member Author

#5727 should have enabled Iterator::size_hint on abi3? 🤔

@Icxolu
Copy link
Contributor

Icxolu commented Jan 12, 2026

Oh, weird it looks like local git history just skipped #5727 even though it had this on in it... Not sure how that happed 🤔 but I fixed it now and the issue went away. Sorry for the noise.

@davidhewitt
Copy link
Member Author

No prob, strange git bug!

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.

3 participants