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.

Sign up for GitHub

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

Jump to bottom

Renaming Agent's methods #3557

Merged
merged 24 commits into from
Mar 9, 2020
Merged

Renaming Agent's methods #3557

merged 24 commits into from
Mar 9, 2020

Conversation

vincentpierre
Copy link
Contributor

Proposed change(s)

  • The following methods in the Agent class were renamed:
    • InitializeAgent() was renamed to Initialize()
    • AgentAction() was renamed to OnActionReceived()
    • AgentReset() was renamed to OnEpsiodeBegin()
    • Done() was renamed to EndEpisode()
    • GiveModel() was renamed to SetModel()

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

Renaming sheet

Types of change(s)

  • [ ] Bug fix
  • [ ] New feature
  • Code refactor
  • Breaking change
  • Documentation update
  • [ ] Other (please describe)

Checklist

  • [ ] Added tests that prove my fix is effective or that my feature works
  • Updated the changelog (if applicable)
  • Updated the documentation (if applicable)
  • Updated the migration guide (if applicable)

Other comments

@vincentpierre
[skip ci] Renamed methods in the Agent class
7bd6e4e 
WARNING, the user when implementing obsolete methods will see the message :Member `old method` overrides obsolete member `old method`. Add the Obsolete attribute to `old method`. It will not suggest the new method to override.
@vincentpierre
[skip ci] Updated the example environment
24dac1b
@vincentpierre
[skip ci] Updated migrating and changelog
d17789d
@vincentpierre
[skip ci] Editing the docs
dbebc42
@vincentpierre
[skip ci] Missing docs
29409b3
@vincentpierre vincentpierre self-assigned this Mar 4, 2020
chriselion
chriselion reviewed Mar 4, 2020
View reviewed changes
Copy link
Contributor

@chriselion chriselion left a comment

Choose a reason for hiding this comment

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

I think it looks pretty good overall.

Please read the context around the doc changes instead of just doing a find-and-replace; I flagged some things but it could use another pair of eyes on it too.

@surfnerd
Copy link
Contributor

surfnerd commented Mar 4, 2020

is it possible to leverage this. I think it would make our users' lives easier if we could.

vincentpierre and others added 4 commits March 4, 2020 11:21
@vincentpierre
Update docs/Getting-Started-with-Balance-Ball.md
675c38f 
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
@vincentpierre
Update docs/Learning-Environment-Create-New.md
0da6957 
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
@vincentpierre
Update docs/Learning-Environment-Create-New.md
c29ac08 
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
@vincentpierre
[skip ci] documentation changes
59f8e19
vincentpierre
vincentpierre commented Mar 4, 2020
View reviewed changes
docs/Getting-Started-with-Balance-Ball.md Outdated Show resolved Hide resolved
docs/Getting-Started-with-Balance-Ball.md Outdated Show resolved Hide resolved
docs/Getting-Started-with-Balance-Ball.md Outdated Show resolved Hide resolved
docs/Getting-Started-with-Balance-Ball.md Outdated Show resolved Hide resolved
@vincentpierre vincentpierre changed the title Develop renaming 001 Renaming Agent's methods Mar 5, 2020
awjuliani
awjuliani approved these changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@awjuliani awjuliani left a comment

Choose a reason for hiding this comment

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

I found a typo, but otherwise looks good.

@vincentpierre
Copy link
Contributor Author

I found a typo, but otherwise looks good.

All right, I will wait for another approval since this is a rather controversial PR.

mmattar
mmattar reviewed Mar 6, 2020
View reviewed changes
com.unity.ml-agents/Runtime/Agent.cs Outdated Show resolved Hide resolved
docs/Migrating.md Outdated Show resolved Hide resolved
docs/Migrating.md Outdated Show resolved Hide resolved
com.unity.ml-agents/CHANGELOG.md Outdated Show resolved Hide resolved
@chriselion chriselion mentioned this pull request Mar 9, 2020
Merged
10 tasks
ervteng
ervteng reviewed Mar 9, 2020
View reviewed changes
docs/Migrating.md Outdated Show resolved Hide resolved
ervteng
ervteng reviewed Mar 9, 2020
View reviewed changes
docs/Migrating.md Outdated Show resolved Hide resolved
@vincentpierre vincentpierre merged commit 4aed4fd into master Mar 9, 2020
@delete-merged-branch delete-merged-branch bot deleted the develop-renaming-001 branch March 9, 2020 21:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mmattar mmattar mmattar left review comments

@awjuliani awjuliani awjuliani approved these changes

@chriselion chriselion chriselion approved these changes

@ervteng ervteng ervteng approved these changes

@surfnerd surfnerd surfnerd approved these changes

@andrewcoh andrewcoh Awaiting requested review from andrewcoh

Assignees

@vincentpierre vincentpierre

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@vincentpierre @surfnerd @mmattar @ervteng @chriselion @awjuliani