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 EnvInteractData with Python3.9 #6

Merged

Conversation

sebimarkgraf
Copy link
Contributor

This fixes #4 by changing the namedtuple to a dataclass.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 29, 2022
@ssnl
Copy link
Collaborator

ssnl commented Sep 29, 2022

Thanks for the proposed fix! Can you change it so that it uses namedtuple still before 3.9? I tried to namedtuple for every case where things are getting created & destroyed in each iteration, and ideally would benefit from the lighter wrapper (of namedtuple compared to dataclass).

@sebimarkgraf sebimarkgraf changed the title Change EnvInteractData to use a dataclass Fix EnvInteractData with Python3.9 Sep 29, 2022
@sebimarkgraf
Copy link
Contributor Author

I've changed it to use the NamedTuple from typing extensions.
In 3.11 this will be officially supported: python/cpython#92027

I am not sure about the performance benefits, this strongly depends on the use case and would probably need profiling here.

@ssnl
Copy link
Collaborator

ssnl commented Sep 29, 2022

Thanks!

@ssnl ssnl merged commit 59015b8 into facebookresearch:main Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: Multiple inheritance with NamedTuple is not supported
3 participants