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 test to handle nested observation dicts #1172

Merged
merged 6 commits into from
Mar 22, 2024

Conversation

dm-ackerman
Copy link
Contributor

Description

The previous version of the test worked for a dict observation, but not a dict of a dict. This fix will allow observations with unlimited nesting to be correctly tested (at least for the specific test that was failing).

Fixes #1167, Depends on #1170

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have run pytest -v and no errors are present.
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I solved any possible warnings that pytest -v has generated that are related to my code to the best of my knowledge.
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

The previous version of the test worked for a dict observation,
but not a dict of a dict. This fix will allow observations with
unlimited nesting to be correctly tested (at least for the specific
test that was failing).
@elliottower
Copy link
Member

This would be good to get merged but it looks like it failed because int has no dtype? Not sure what that is about

@dm-ackerman
Copy link
Contributor Author

Tests are failing due to AgileRL version change. #1193 should resolve that.

Copy link
Member

@elliottower elliottower left a comment

Choose a reason for hiding this comment

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

Looks good to me seems to be passing now too

pettingzoo/test/api_test.py Outdated Show resolved Hide resolved
pettingzoo/test/api_test.py Outdated Show resolved Hide resolved
@elliottower elliottower merged commit 0cdf49e into Farama-Foundation:master Mar 22, 2024
47 checks passed
@dm-ackerman dm-ackerman deleted the bugfix_1167 branch March 22, 2024 15:12
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.

[Bug Report] API test failing for nested observations
3 participants