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

Type cast in spaces families #2491

Merged
merged 5 commits into from
Dec 16, 2021
Merged

Type cast in spaces families #2491

merged 5 commits into from
Dec 16, 2021

Conversation

XuehaiPan
Copy link
Contributor

@XuehaiPan XuehaiPan commented Nov 17, 2021

1. spaces.Dict

in __init__:

  • convert tuple to OrderedDict as well (same as list).
  • raise AssertionError when the user does not pass a dict or list / tuple of keyvalue pairs.

2. spaces.Tuple:

in __init__:

  • convert input spaces to tuple.

in contains:

  • convert the input sample to tuple.

Fixes #2140.
Fixes #2479.


3. spaces.MultiBinary

in __init__:

  • convert input shapes to tuple of ints.
  • raise AssertionError when any entry of the input shapes is not positive.

4. spaces.MultiDiscrete

in __init__:

  • make a copy of the input nvec.

    >>> import numpy as np
    >>> a = np.array([1, 2, 3], np.int64)
    >>> np.asarray(a) is a
    True
    
    >>> np.asarray(a)[0] = 4
    >>> a
    array([4, 2, 3])
  • raise AssertionError when any entry of the input shapes is not positive (convert to ints first).

in contains:

  • convert tuple to np.ndarray as well (same as list).

5. spaces.Discrete

in __init__:

  • convert the input shape int.
  • raise AssertionError when the input shape is not positive (convert to ints first).

Copy link
Contributor

@RedTachyon RedTachyon left a comment

Choose a reason for hiding this comment

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

Overall looks good, one point around the code in this PR - do we need the OrderedDicts in spaces.Dict? Since Python 3.7 (i.e. the earliest version that's actually supported by gym), dicts are guaranteed to be ordered, so I'd probably prefer just making everything into a dict. Not sure if we need to do that in this PR though.

One other minor comment left as review

gym/spaces/dict.py Outdated Show resolved Hide resolved
@jkterry1
Copy link
Collaborator

@RedTachyon are you happy with it now?

@RedTachyon
Copy link
Contributor

@jkterry1 I think can be merged now, there's still a conversation to be had about dict vs OrderedDict, but that can be done in another PR

@jkterry1 jkterry1 merged commit 18c8b98 into openai:master Dec 16, 2021
@jkterry1
Copy link
Collaborator

@RedTachyon can you please create an issue regarding the OrderedDict thing so it doesn't get forgotten?

@XuehaiPan XuehaiPan deleted the spaces-type-cast branch December 16, 2021 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants