-
Notifications
You must be signed in to change notification settings - Fork 30
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
Improve docstring and code documentation #355
Conversation
for more information, see https://pre-commit.ci
rlberry/agents/agent.py
Outdated
@@ -209,6 +204,7 @@ def fit(self, budget: int, **kwargs): | |||
@abstractmethod | |||
def eval(self, **kwargs): | |||
""" | |||
Abstract method to be overwriten by the 'inherited agent' developer. |
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.
Abstract method for what ?
rlberry/agents/agent.py
Outdated
@@ -485,32 +490,74 @@ class AgentWithSimplePolicy(Agent): | |||
|
|||
@abstractmethod | |||
def policy(self, observation): | |||
"""Returns an action, given an observation.""" | |||
""" | |||
Abstract method to be overwriten by the 'inherited agent' developer. |
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.
I can smell ChatGPT, can this be simplified (I think there is a lot of repetition)?
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.
Indeed, some parts were generated automatically, should be cleaner now
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.
If you talk about the "Abstract method to be overwriten by the 'inherited agent' developer." it's not ChatGPT, but me.
It's because the decorator because the decorator didn't appear in the documentation. (and the function do nothing)
If you talk about the rest of the text I don't know.
@@ -584,11 +636,8 @@ class AgentTorch(Agent): | |||
""" | |||
|
|||
def save(self, filename): | |||
# Overwrite the 'save' method to manage CPU and GPU with torch agent |
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.
Why this is not in the docstring?
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.
I move it outside the docstring, because it's not really useful for the user, only for the developer. But I can put it in back if we think that was better before.
Co-authored-by: Waris Radji <[email protected]>
Co-authored-by: Waris Radji <[email protected]>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…MLP part of the CNN
for more information, see https://pre-commit.ci
In this PR, you could also search & replace all the :class: |
c'est fait, bien vu! |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Hello, |
Hello, |
Hello, |
updated here : |
#342
Description
Checklist