Checked this implementation against the old one and it makes the test pass without causing any other ones to fail

Thanks! Do you mind updating all the files in collections? As far as I can tell, this just updates the __init__.py file, right? (feel free to ignore if you updated all and theres just no diff) It would be best if the PR updated them all so we can have all files be in sync. (See #4564 for reference here).

We can always merge and update the others later but this way we get two birds with one stone.

As far as I can tell there's nothing to be changed in either collections/abc.py or collections/defaultdict.py so I think we're good? Unless I've made another blunder which is very possible 😅

Copying __init__.py over from the 3.11 branch (specifically 3.11.2) shows a couple more changes that have been made to it (some in OrderedDicts __reduce__, some shifting of methods in Counter and some tiny comment changes). Maybe you've copied from an older version?

Note: One of these is a FIXME added by us, don't remove that (basically, add it back in after you copy __init__.py over) since its still required 😄

So, I think the best approach now is to include those changes from 3.11 here, rename the PR to Update collections from Python 3.11.2 and we can see if the test suite picks anything else up.

Thanks again for opening, feel free to ask if anything gets confusing.

Made the suggested changes. Thanks for all the help :)

Didn't realise so much of the ethos of this project is about staying in line with the CPython implementations (which of course reflecting on it makes total sense)!

Just checking the tests now and it seems like actually these changes fix another test that was failing!

___________ PurePythonOrderedDictWithSlotsCopyingTests.test_copying ____________
Unexpected success

If we're happy that this test should be passing then I'll go ahead and take away the decorator :)

I'm going to guess it most likely isn't happy with 8e55919 which seems kinda odd considering your output. Since the runner's thirst for success must be appeased we should rollback that commit and give it a second try to make sure.

I think that's done? Still working on my git-fu

Had a dig through the CI logs and it seems like the following tests are failing in a number of test classes:

• test_copying
• test_pickle_recursive
• test_yaml_linkage
• test_reduce_not_too_fat

I can't seem to replicate the errors with test_pickle_recursive, test_yaml_linkage, test_reduce_not_too_fat, but on my end test_copying failing just seems to be due to the same decorator as in 8e55919. (I guess this makes sense seeing as the mac os tests go through fine and I'm on mac os)

Okay, failures do make sense, they all are a result of the new __reduce__ for OrderedDict (which calls __setstate__ which is nowhere to be found). This sometimes happens, library is updated, uses a feature we might not have, tests that worked fine with the old implementation now fail.

You can now mark these as expected failures and I should probably see what is the deal with __setstate__.

Before I add those decorators, would it be worth removing all of the decorators and rerunning the workflows just so everything's fully up to date and we can see which tests are going through according to the CI?

Also seems like an issue with __getstate__ too? Not sure why the tests are only failing on ubuntu and not on mac os though? The code for __reduce__ makes a call to __getstate__() immediately, but, like you, I can't find a definition for __getstate__() on anything dictionary related yet the tests still pass.

Not sure why the tests are only failing on ubuntu and not on mac os though?

Don't think they are executed on Mac. These are platform independent (don't really test any platform specific things) so having them executed only on Ubuntu (fastest runner) is a solution to cut down on unnecessary execution time.

Not sure why the tests are only failing on ubuntu and not on mac os though?

Don't think they are executed on Mac. These are platform independent (don't really test any platform specific things) so having them executed only on Ubuntu (fastest runner) is a solution to cut down on unnecessary execution time.

Fair enough - makes sense. Still not sure why this test passes on my laptop though.

Before I add those decorators, would it be worth removing all of the decorators and rerunning the workflows just so everything's fully up to date and we can see which tests are going through according to the CI?

Opinions on this idea? Probably worth making sure that we know which tests are now passing and/or failing?

Before I add those decorators, would it be worth removing all of the decorators and rerunning the workflows just so everything's fully up to date and we can see which tests are going through according to the CI?

Opinions on this idea? Probably worth making sure that we know which tests are now passing and/or failing?

It would be a good idea to remove RustPython-specific @unittest.skip() decorators to see if they still need to be skipped. The @unittest.expectedFailure decorators don't need to be removed.

I can't seem to replicate the errors with test_pickle_recursive, test_yaml_linkage, test_reduce_not_too_fat

I believe you can use the following command to test your changes locally:

# this runs all of the tests (not necessary if you know which test is affected)
cargo run --release --features ssl,jit -- -m test -j 1 -u all --slowest --fail-env-changed -v
# this runs only the test suite named "test_whatever" (usually located at `Lib/test/test_whatever.py`)
cargo run --release --features ssl,jit -- -m test -j 1 -u all --slowest --fail-env-changed -v test_whatever

For a list of all resources and more command-line options, you can execute cargo run --release --features ssl,jit -- -m test -h.

Ahhhh thank you for this!

Just updated the decorators so now all the tests should pass the CI.

re-run of ci won't fly well with the recent windows ci changes it seems @Phosphorescentt can you rebase?

Thank you!