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

Add figures #35

Merged
merged 10 commits into from
Mar 23, 2022
Merged

Add figures #35

merged 10 commits into from
Mar 23, 2022

Conversation

lvwerra
Copy link
Member

@lvwerra lvwerra commented Mar 23, 2022

This PR adds all the figures to the notebooks (except for chapter 2 which already has figures).

@lvwerra lvwerra requested a review from lewtun March 23, 2022 00:07
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Member

@lewtun lewtun left a comment

Choose a reason for hiding this comment

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

Thanks a lot for adding the images - looks great!

Do you think we should hold off on merging this PR until we've triaged / merged the current open ones? I'm a bit worried that we'll get horrible merge conflicts for the open PRs and not everyone is familiar with nbdev

@lvwerra
Copy link
Member Author

lvwerra commented Mar 23, 2022

I looked at the other PRs and they are all clean (1-2 line changes). Worst case we just apply nbdev and commit to the PR, no? I am more worried fixing this PR than the others :)

@lewtun
Copy link
Member

lewtun commented Mar 23, 2022

Haha ok I like your approach - merging this beast now!

@lewtun lewtun merged commit 3ca7b1f into main Mar 23, 2022
@lewtun lewtun deleted the add-figures branch March 23, 2022 17:42
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.

2 participants