[Proposal] Ensure seed_test checks for action masks in both observation and info dict

[Kchour]

Proposal

In line 50-51 of pettingzoo.test.seed_test (see link), the current method of getting an action mask may not be 100% consistent with documentation.

The documentation says that action masking can be stored in either mask = observation["action_mask"] or mask = info["action_mask"], but the above seed test only checks for masks under the former. It may be best to check for the action mask in both places. Thus, I propose we change lines 50-51 to the following, which checks for a mask in both the observation and info dicts:

mask1 = obs1.get("action_mask") if (isinstance(obs1, dict) and "action_mask" in obs) else (info1.get("action_mask) if "action_mask" in info1 else None)
mask2 = obs2.get("action_mask") if (isinstance(obs2, dict) and "action_mask" in obs) else (info2.get("action_mask) if "action_mask" in info2 else None)

The code comes directly from the pettingzoo documentation on AEC API

Motivation

The above feature will help the seed_test's action masking feature be more in line with the documentation since it is clearly stated in the documentation:

Note: action masking is optional, and can be implemented using either observation or info.

Otherwise, there should be more emphasis on doing things one way or the other.

Pitch

Discuss the viability of the feature and integrate into the codebase

Alternatives

I guess we can tell everyone to simply stick with putting action masks in one place only...

Additional context

I was putting together a custom pettingzoo environment but it kept failing the seed test. Upon further inspection, realized that the action mask was not being used since I was storing them within info, which I find to be cleaner.

Checklist

• I have checked that there is no similar issue in the repo

[elliottower]

Thanks for catching this, we've needed to do a PR like this for a while but it's a lot of effort because there are many other places in the codebase where action masking occurs (e.g., in the different wrappers or conversions) and I think they should all be done together, but at the very least the API test should be consistent as you point out.

[elliottower]

Tried to give you co-authorship but I think the email associated with your most recent commit (a year or two ago, looked through your repos) was incorrect:

Co-authored-by: Kenny <[email protected]>

I'll try to tag you in the release notes at least, sorry about the mismatch. Seems to link to this account https://github.com/Kennjaro for some reason? Maybe you have two accounts and your git credentials are mismatched or something