Add basic CI to test documentation (using pytest-markdown-docs)

[elliottower]

Talked with Rodrigo a bit about this, figured I'd throw up a PR for it in case you guys want it. The plugin pytest-markdown-docs isn't the most fully featured library and can be somewhat slow if documentation gets very large, but does work out of the box pretty well. I added it to PettingZoo and it works well and has showed me a number of documentation issues.

It looks like there are two issues it detected currently:
ERROR docs/_scripts/gen_dataset_md.py - gymnasium.error.VersionNotFound: Environment version `v4` for environment `AntMaze_Large_Diverse_GR` doesn't exist. It provides versioned environments: [ `v3` ].

docs/tutorials/dataset_creation/point_maze_dataset.py:305: in <module>
    obs, rew, terminated, truncated, info = collector_env.step(action)
minari/data_collector/data_collector.py:216: in step
    assert self.dataset_action_space.contains(
E   AssertionError: Actions are not in action space.

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue), Depends on # (pull request)

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.
To upload images to a PR -- simply drag and drop or copy paste.

Checklist:

• I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
• I have run pytest -v and no errors are present.
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I solved any possible warnings that pytest -v has generated that are related to my code to the best of my knowledge.
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[younik]

LGTM, we must solve the issues in the documentation

[elliottower]

I created this in the repo not a fork so you guys can edit the branch and fix the bugs

[elliottower]

Looks like these changes are correct to me but what’s weird now is the documentation test action took an hour then cancelled after your first change, then six hours and cancelled after the second.

Maybe there’s training code that is in the docs but should be excluded from these tests? Like the torch data loader tutorial.

[elliottower]

I believe the default pytest ignore syntax works here so you can in line —ignore=docs/tutorials/myscript.py

[elliottower]

This error is still there: E gymnasium.error.VersionNotFound: Environment version `v4` for environment `AntMaze_Large_Diverse_GR` doesn't exist. It provides versioned environments: [ `v3` ].

[elliottower]

This error is still there: E gymnasium.error.VersionNotFound: Environment version `v4` for environment `AntMaze_Large_Diverse_GR` doesn't exist. It provides versioned environments: [ `v3` ].

This errors out for me when I run it locally but I'm not seeing it in the CI, honestly not sure why the CI seems to be taking forever, maybe the move is to just fix these issues that have been found because of it but not actually put it in the CI unless the runtime can be made shorter (cause you definitely don't want a longer iteration time because of one test, seems like this bit runs for over an hour sometimes?)

[elliottower]

On my laptop (6 core i7 intel 2019 mbp 16 inch) it takes 5 minutes for reference

[younik]

This error is still there: E gymnasium.error.VersionNotFound: Environment version `v4` for environment `AntMaze_Large_Diverse_GR` doesn't exist. It provides versioned environments: [ `v3` ].

I don't see it in my laptop as well; which version of gymnasium-robotics do you have?

[elliottower]

This error is still there: E gymnasium.error.VersionNotFound: Environment version `v4` for environment `AntMaze_Large_Diverse_GR` doesn't exist. It provides versioned environments: [ `v3` ].

I don't see it in my laptop as well; which version of gymnasium-robotics do you have?

Ohhh it must be an old version, maybe you should require the latest version in the testing extra (I believe I did pip install -e .[testing] —u to upgrade but maybe not

[younik]

This error is still there: E gymnasium.error.VersionNotFound: Environment version `v4` for environment `AntMaze_Large_Diverse_GR` doesn't exist. It provides versioned environments: [ `v3` ].

I don't see it in my laptop as well; which version of gymnasium-robotics do you have?

Ohhh it must be an old version, maybe you should require the latest version in the testing extra (I believe I did pip install -e .[testing] —u to upgrade but maybe not

Oh you are right, the v4 was introduced with 1.2.3, but the dependency are requiring >=1.2.1; I fix this

[elliottower]

Btw I found this plugin which apparently supports shell commands as well as Python so maybe we could have it even test the stuff like CLI calling minari list datasets or whatever, also had a pr merged and new release on sept and has more stars than the other one, so maybe it’s better https://github.com/nschloe/pytest-codeblocks

[younik]

On my laptop (6 core i7 intel 2019 mbp 16 inch) it takes 5 minutes for reference

On my laptop (Apple M1) it takes more than 30 mins. Are you using pytest docs --markdown-docs -m markdown-docs?

Btw, it is failing exactly because bash codes are not run (needed for this tutorial: https://minari.farama.org/tutorials/using_datasets/behavioral_cloning/), so I think it is better to migrate to the library you suggested

[elliottower]

I don’t have too much time to mess around with that other library, if one of you guys wanted to that would probably be best (as I also don’t know which bash scripts etc are needed to execute)

[elliottower]

Thanks for handling this Omar, weird that it still errored out on docs/tutorials things even though it says exclude in the pyproject.toml

[younik]

Thanks for handling this Omar, weird that it still errored out on docs/tutorials things even though it says exclude in the pyproject.toml

I actually removed it before, to try using it inline, but apparently it didn't work.
Now I am testing the tutorials with nbmake, and the markdown files with pytest-markdown-docs. I still need to solve two problems, then it should be good.

[younik]

@elliottower can you check if my changes make sense to you? Thanks

[elliottower]

@elliottower can you check if my changes make sense to you? Thanks

Can't approve cause it's my own PR but looks good to me

[elliottower]

Wait hold on it looks like every single test on here is done twice because of "push" and "pull request". Push should only be to the master branch once it is merged. I'll go check on that.

[elliottower]

Oh oops I created this PR from the main repo's branch not a fork, so it seems to not be running the CI now because I set it to only do the master branch, I'll try to fix that

[elliottower]

There we go now it only does 6 actions and doesn't duplicate it

[elliottower]

LGTM now, I made a separate PR as well just adding some additional pre-commit checks that I was going to put here (on the topic of making CI match gymnasium/pz) but didn't want to muddy things and change too many files with one PR.

Edit: checked the duration of each test and the new docs test now slows down the iteration time from ~3-5 minutes to 11 minutes, I'm testing pytest-xdist because it should be able to roughly half the execution time using multiprocessing. Only added it to that test though as it's not necessary to run for everything.

Edit 2: Looks like that didn't work because of some async h5py file opening stuff, guess it's probably fine to have 11 mins for the CI if it does actually check all the docs

[elliottower]

Found a small typo using master instead of main, and still having some stuff from pytest xdist, now I think the PR should be fully good (just reviewed it all)

[elliottower]

Looks like some flake8 issue? Not sure what that’s from

[younik]

I solved the flake8 issues and added the version to minigrid

[younik]

Should we keep this for push on main? @elliottower
I noticed it because we don't have anymore build-publish on merged PRs

[elliottower]

Oh I guess gymnasium also does it for all prs I guess it’s just “can this be built” and only uploads if it’s a release. My bad about that, I don’t think it’ll be a huge deal but probably best to be consistent. Pettingzoo it was just for release but since this is much more based on gymnasium obviously we should inherit from there