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

Attributes docstrings #188

Open
TimotheeMathieu opened this issue May 11, 2022 · 4 comments
Open

Attributes docstrings #188

TimotheeMathieu opened this issue May 11, 2022 · 4 comments
Assignees
Labels
documentation Improvements or additions to documentation Marathon To do during Marathon

Comments

@TimotheeMathieu
Copy link
Collaborator

For now there is practically no attribute documentation except in Agent and AgentWithSimplePolicy. It would be nice to have this for every agent. and every environment also.

@KohlerHECTOR KohlerHECTOR added documentation Improvements or additions to documentation Marathon To do during Marathon labels Jul 13, 2023
@KohlerHECTOR KohlerHECTOR self-assigned this Jul 24, 2023
@JulienT01 JulienT01 assigned JulienT01 and unassigned JulienT01 Jul 24, 2023
KohlerHECTOR added a commit that referenced this issue Jul 24, 2023
@KohlerHECTOR
Copy link
Collaborator

PR: #349 : Done with DQN-PPO-REINFORCE.
To do torch: A2C-MDQN.
To do non-torch: all of them.

@KohlerHECTOR
Copy link
Collaborator

To do torch: A2C-MDQN.
To do non-torch: all of them.

@riccardodv
Copy link
Collaborator

So far, we have put all attributes. These are often just a copy and paste of the parameters, leading to some redundancy. I understand it could be helpful to have both things, but do you think this is the best way to do it? The docstring will be quite long in this way.

@TimotheeMathieu
Copy link
Collaborator Author

Maybe it would be sufficient to document the attributes that are not arguments but created during the fit. A bit like in scikit-learn ;) . in the case of scikit-learn, the attributes that could be reused by the user are denoted with a "" at the end (e.g. regressor.coef ) they say "an attribute that have a meaningful value after fit() was called". Maybe we could take the same convention for rlberry agents ?

riccardodv added a commit to riccardodv/rlberry that referenced this issue Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation Marathon To do during Marathon
Projects
None yet
Development

No branches or pull requests

4 participants