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

MAINT: move non-executable content to its own category. #148

Merged
merged 2 commits into from
Jan 16, 2023

Conversation

rossbar
Copy link
Collaborator

@rossbar rossbar commented Sep 8, 2022

As discussed in the previous documentation meeting, this PR implements the "medium-term" solution to the tutorials that have been added but are not currently executable.

Essentially, this moves the RL and NLP tutorials to a separate category that I called "articles" (naming suggestions welcome) and adds some text that these are not tested. I've also added admonitions soliciting contributions from any one who might be interested in helping to make them fully executable.

@bsipocz
Copy link
Member

bsipocz commented Sep 21, 2022

Essentially, this moves the RL and NLP tutorials to a separate category that I called "articles" (naming suggestions welcome) and adds some text that these are not tested. I've also added admonitions soliciting contributions from any one who might be interested in helping to make them fully executable.

What are the limitations on making them executable? Too many dependencies/resource requirements for being practical, or some other reasons?

@rossbar
Copy link
Collaborator Author

rossbar commented Oct 4, 2022

What are the limitations on making them executable? Too many dependencies/resource requirements for being practical, or some other reasons?

For the reinforcement learning tutorial, it's a dependency issue - specifically licensing of ROMs which have EULA's/potential licensing concerns. See e.g. #87 for more details.

For the NLP tutorial it was mostly a matter of data hosting which was the bottleneck, then the tutorial was merged provisionally with the intention of following up once a tutorial data hosting solution had been worked out. Since it wasn't executable when merged, this one would also need review.

@bsipocz
Copy link
Member

bsipocz commented Oct 4, 2022

Oh, wonderful.

@rossbar
Copy link
Collaborator Author

rossbar commented Oct 31, 2022

@melissawm just a quick ping here if you have any thoughts on this reorganization proposal!

Copy link
Member

@melissawm melissawm left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, this looks good to me. I wonder if we should expand on the reason why these tutorials are not executable at the index page, too. We could just copy the

This article is not currently tested due to licensing/installation issues with
the underlying `gym` and `atari-py` dependencies.
Help improve this article by developing an example with reduced dependency
footprint!

message from within the tutorial to the index page.

@rossbar
Copy link
Collaborator Author

rossbar commented Jan 16, 2023

Thanks all - I'll go ahead and get this in. Once the categories are separated, if anyone would like to add/adjust the admonitions, please follow-up with an issue/PR!

@rossbar rossbar merged commit a6df091 into numpy:main Jan 16, 2023
@rossbar rossbar deleted the separate-non-exec-content branch January 16, 2023 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants