-
Notifications
You must be signed in to change notification settings - Fork 28
Get rid of deprecation warnings in tests and examples #63
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
Conversation
examples/human.py
Outdated
|
||
elif (userInputStr == 'goals'): | ||
print(env.getGoalProgressStr()) | ||
print(env.get_goal_progress_srt()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo here ..._srt
vs. ..._str
. But fundamentally, do we need it in the first place? I'd be in favor to drop the _str
all together. The docstring should be clear enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh oops. Somehow that ran on my machine, even with the typo. I'm fine with that. My policy was to just keep the method the calls exactly the same, but I'm happy to change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Just make sure to make the change in scienceworld.py too.
Re: testing, in order to trigger this, you need to run the script and type "goals" as one of the in-game actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or if you prefer, we cn make another PR that removes all those type from the variable/method names. What do you think @AndKaminer ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, get_goal_progress_str is the only method where removing the end type doesn't get rid of any important information, so I think w get rid of just that one.
I changed the methods from the camel case variants to snake case.