Skip to content

Conversation

@jbischof
Copy link
Contributor

No description provided.

Copy link
Member

@mattdangerw mattdangerw left a comment

Choose a reason for hiding this comment

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

Just leaving some random rendering comments I noticed when generating this.

Copy link
Contributor

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@jbischof jbischof marked this pull request as ready for review December 21, 2022 01:16
@jbischof jbischof requested a review from MarkDaoust as a code owner December 21, 2022 01:16
@jbischof jbischof changed the title Add quickstart guide for KerasNLP Add "Getting Started" guide for KerasNLP Dec 21, 2022
Copy link
Member

@mattdangerw mattdangerw left a comment

Choose a reason for hiding this comment

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

Just did a pass on some of the hyperlinks!

Copy link
Contributor

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

The images appear to be pixellated (the text below them especially). Can you generate clean ones? Also, no need to add them to the git tree (in general it should be avoided since it increases the size of the repo, which we're constantly downloading), you can put them on imgur.

@jbischof
Copy link
Contributor Author

@fchollet imgur seems a little weird. What if we used our GCP bucket? I don't have access to our the keras-io bucket but here is an example upload to a test bucket (link).

Copy link
Contributor

@fchollet fchollet left a comment

Choose a reason for hiding this comment

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

Looking good! Please add the generated files.

Please note that I've pushed minor copyedits (in particular, using absolute URLs for links, which is necessary since many folks use the tutorial interactively via the Colab link where relative URLs wouldn't work). Pull them first.

@mattdangerw
Copy link
Member

(in particular, using absolute URLs for links, which is necessary since many folks use the tutorial interactively via the Colab link where relative URLs wouldn't work)

Oops, my bad! I asked for this. Thought relative would be better for previews, but the colab issue makes sense.

@fchollet fchollet merged commit 2481e80 into keras-team:master Dec 28, 2022
@innat
Copy link
Contributor

innat commented Aug 7, 2023

What a well-written guide! Love it.
Thanks @jbischof 👍

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.

4 participants