Farama-Foundation / PettingZoo

An API standard for multi-agent reinforcement learning environments, with popular reference environments and related utilities
https://pettingzoo.farama.org
Other
2.45k stars 400 forks source link

Fix test failing due to __getattr__ in module #1175

Closed dm-ackerman closed 4 months ago

dm-ackerman commented 4 months ago

Description

The deprecated_handler() function is called by module level getattr functions. It raised an ImportError when a module wasn't an env. This breaks getattr's behaviour when it is called with a default value. It expects getattr to raise an AttributeError if the attr is not found.

Pytest was apparently updated to call getattr(obj, key, default) for several values of key. The import error isn't caught by getattr, breaking the test.

Changing ImportError to AttributeError fixes this. The missing attribute raises the correct error, which is caught by getattr, which sets the value to the default and everything works as intended.

Type of change

Checklist:

dm-ackerman commented 4 months ago

Well that's annoying, doc tests fail.

It works locally, but seems the workflow uses a different test command. This appears to be a problem with pytest-markdown-docs being incompatible with version 8 of pytest. A random person posted a fix and I opened an issue with pytest-markdown-docs asking them to add the changes and publish a new version. They were super responsive and looks like they are checking it out now. Hopefully it gets approved and sent out.

I patched pytest-markdown-docs locally and it fixes the error. So I'm optimistic that if they push an update, this test failure will be solved.

dm-ackerman commented 4 months ago

The pytest-markdown-docs folks were amazing and got pytestv8 support all sorted out. I upgrade the version and it works locally after correcting some problems it found in docs. Added a PR with those fixes, in addition to these, to hopefully be fully compatible with pytest v8: #1177 Closing this. Use that instead.