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

Update notebook table and transformers intro notebook #9136

Merged
merged 2 commits into from
Dec 16, 2020
Merged

Conversation

sgugger
Copy link
Collaborator

@sgugger sgugger commented Dec 15, 2020

What does this PR do?

Update the examples table and the notebooks table to include all recent examples. Also fix the intro notebook to the transformers library, in particular, the image that was missing.

Fixes #9083

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

LGTM, clearer this way!

@@ -89,13 +91,12 @@
},
"outputs": [],
"source": [
"!pip install transformers\n",
"!pip install --upgrade tensorflow"
"# !pip install transformers"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth it to pin transformers to the current version since we don't test the notebooks -> !pip install transformers==4.0.0 . Wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The goal is still to have those work on the latest release. I'm afraid the pinning will be forever left there afterward and the notebook will never be updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, yeah, can we add a test to make sure the notebooks always work with the latest version? I'm a bit worried that we'll just forget to update the notebook and it doesn't work which will churn away many users

Copy link
Collaborator Author

@sgugger sgugger Dec 16, 2020

Choose a reason for hiding this comment

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

The problem is most of the notebooks all have a big training, which takes too long to run. It's easy to have something that execute a given notebook, but if we have to skip some cells because of the time it takes, we usually can't execute the cells after.

So we can automate testing on those that take not too long (might require a machine with a GPU) but not the bigger ones.

Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Nice!

@sgugger
Copy link
Collaborator Author

sgugger commented Dec 16, 2020

Discussion on pinning/testing notebooks needs to be global on all notebooks (not just one) so merging this for now. We can think of a strategy and implement it in a follow-up PR.

@sgugger sgugger merged commit 4d48973 into master Dec 16, 2020
@sgugger sgugger deleted the notebook_page branch December 16, 2020 15:24
guyrosin pushed a commit to guyrosin/transformers that referenced this pull request Jan 15, 2021
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.

Image rendering not working in example notebook
3 participants