-
Notifications
You must be signed in to change notification settings - Fork 235
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
CLI documentation #743
CLI documentation #743
Conversation
One thing worth mentioning (discussed yesterday with Adam) is that if an environment is registered upon importing a package, or in a certain file (e.g. inside Some information about this functionality is in the Gymnasium docs. The same thing worked in gym==0.21.0 as well |
Thanks for letting me know! This will also benefit the rl-baselines3-zoo! I will add an example with this to the documentation. Note: depends on huggingface/huggingface_sb3#32 to go upstream because environment names with the package prefix are not properly normalized as of now. |
Codecov Report
@@ Coverage Diff @@
## master #743 +/- ##
=======================================
Coverage 96.33% 96.33%
=======================================
Files 93 93
Lines 8789 8789
=======================================
Hits 8467 8467
Misses 322 322
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Looks really good. A couple of minor suggestions. Ping me for a rereview when you're ready.
|
||
Note: the cartpole environment is specified via a named configuration | ||
|
||
.. code-block:: bash |
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.
Can we make these some kind of doc test so that they run during the build phase? I'm worried they will end up getting out of sync with the CI over time.
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.
You are right with this concern!
Unfortunately this is not so easy to do. See https://stackoverflow.com/questions/71497528/can-i-modify-doctest-to-test-bash-inside-sphinx-doc
We would either have to use python code and run the commands via subprocess or use some alpha-stage python module also does not quite exactly do what we need (we would have to re-write the documentation in Markdown).
Given that the CLI is going to change soon anyway, I would not put in the effort to test it right now. What do you think?
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.
The python module in question: https://github.com/jamesbehr/docshtest
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.
Sounds good lets just make sure to add a task to the issue tracker for the CI overall to update this part of the documentation.
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 added it to the bottom of the description here: #703
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.
LGTM
|
||
Note: the cartpole environment is specified via a named configuration | ||
|
||
.. code-block:: bash |
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.
Sounds good lets just make sure to add a task to the issue tracker for the CI overall to update this part of the documentation.
# Conflicts: # src/imitation/scripts/ingredients/demonstrations.py
Description
Add some documentation for the command line interface.
Testing
Documentation only changes.