Skip to content

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

Merged
merged 7 commits into from
Dec 20, 2023

Conversation

AndKaminer
Copy link
Contributor

I changed the methods from the camel case variants to snake case.


elif (userInputStr == 'goals'):
print(env.getGoalProgressStr())
print(env.get_goal_progress_srt())
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

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.

Copy link
Collaborator

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 ?

Copy link
Contributor Author

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.

@AndKaminer AndKaminer requested a review from MarcCote December 19, 2023 17:34
@MarcCote MarcCote merged commit 3c90216 into allenai:main Dec 20, 2023
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.

2 participants