Fix deserializing set and frozenset.

[LilyFirefly]

Adding these tests shows the following errors:

failures:

---- de::test::test_tuple_from_pyfrozenset stdout ----
thread 'de::test::test_tuple_from_pyfrozenset' panicked at 'called `Result::unwrap()` on an `Err` value: UnexpectedType("'frozenset' object cannot be converted to 'Sequence'")', src/de.rs:441:46
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- de::test::test_tuple_from_pyset stdout ----
thread 'de::test::test_tuple_from_pyset' panicked at 'called `Result::unwrap()` on an `Err` value: UnexpectedType("'set' object cannot be converted to 'Sequence'")', src/de.rs:441:46


failures:
    de::test::test_tuple_from_pyfrozenset
    de::test::test_tuple_from_pyset

[LilyFirefly]

This branch shouldn't happen for set or frozenset, since the length is known.

It might not be necessary to have the index checks any more, as long as we still check the length is finite.

[LilyFirefly]

It might be appropriate to rename this too, perhaps to PyIteratorAccess.

[codecov-commenter]

Codecov Report

Patch coverage: 88.88% and project coverage change: +0.40 🎉

Comparison is base (cfce252) 80.99% compared to head (858a3fb) 81.40%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #39      +/-   ##
==========================================
+ Coverage   80.99%   81.40%   +0.40%     
==========================================
  Files           4        4              
  Lines        1021     1038      +17     
==========================================
+ Hits          827      845      +18     
+ Misses        194      193       -1     

Impacted Files	Coverage Δ
src/de.rs	88.94% <88.88%> (+0.51%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[davidhewitt]

Thanks for this! This seems reasonable, though I'm also slightly cautious. What's the use case for this? I'm split down the middle on whether allowing set / frozenset as inputs to rust sequences makes sense. E.g. set/frozenset are unordered containers, but we are converting them to an ordered type?

To be fair, serde_json looks like it's doing the same e.g. HashSet becomes a JSON list.

[LilyFirefly]

Thanks for this! This seems reasonable, though I'm also slightly cautious. What's the use case for this?

While I was trying to work out what the best way to handle my use case (see #38) I discovered that the set and frozenset code paths give an unexpected error. So this PR is avoiding that error in the way that fit with what the code seemed to be trying to do.

I'm split down the middle on whether allowing set / frozenset as inputs to rust sequences makes sense. E.g. set/frozenset are unordered containers, but we are converting them to an ordered type?

Yeah - the alternatives I can think of are:

• Remove the special handling and have it just error out like it does with custom python types.
• Treat it as a newtype_struct (https://serde.rs/data-model.html#types).

To be fair, serde_json looks like it's doing the same e.g. HashSet becomes a JSON list.

Yeah, the serde docs (https://serde.rs/data-model.html#types) suggest this handling too, so it's consistent with rust stuff in general. But it does lose a bit of information, so I'm also not certain what's best.

[LilyFirefly]

For my use case, an error immediately with #38 or something with newtype_struct (I'm really not sure exactly what this would look like) would probably be most useful.