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.

Sign up for GitHub

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

Jump to bottom

Rectified typos & grammar in docs #3021

Merged
merged 3 commits into from
Apr 25, 2023
Merged

Rectified typos & grammar in docs #3021

merged 3 commits into from
Apr 25, 2023

Conversation

Neilblaze
Copy link
Contributor

What does this PR do?

A few typos and grammatical errors were discovered and have been corrected.
/cc: @8bitmp3, @marcvanzee

Checklist

  • This PR fixes a minor issue (e.g.: typo or small bug) or improves the docs (you can dismiss the other
    checks if that's the case).
  • The documentation and docstrings adhere to the
    documentation guidelines.

@Neilblaze
patch-fix-docs
fb4d42b 
Signed-off-by: Pratyay Banerjee <putubanerjee23@gmail.com>
cgarciae
cgarciae reviewed Apr 10, 2023
View reviewed changes
docs/guides/batch_norm.rst
@@ -106,17 +106,7 @@ collection.
:sync:

FrozenDict({

Copy link
Collaborator

Choose a reason for hiding this comment

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

These lines are empty intentionally.

cgarciae
cgarciae reviewed Apr 10, 2023
View reviewed changes
docs/guides/dropout.rst
@@ -109,8 +109,6 @@ In short, :meth:`flax.linen.Module.make_rng` *guarantees full reproducibility*.
def __call__(self, x):
x = nn.Dense(self.num_neurons)(x)


Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here.

@cgarciae
Copy link
Collaborator

Hey @Neilblaze, thanks for doing this!

There are a couple of empty lines in .rst files which are intentional as they align two pieces of code in a diff view.

@Neilblaze
Copy link
Contributor Author

@cgarciae rolling back changes for those two files. I also see a couple of empty lines in ../advanced_topics/linen_upgrade_guide.rst & ../guides/linen_upgrade_guide.rst too. Should I also rollback the changes for them too?

@cgarciae
Copy link
Collaborator

Yeah, please rollback anything in either side of a codediff block.

@Neilblaze Neilblaze requested a review from cgarciae April 10, 2023 19:33
cgarciae
cgarciae approved these changes Apr 10, 2023
View reviewed changes
Copy link
Collaborator

@cgarciae cgarciae left a comment

Choose a reason for hiding this comment

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

Looks good, thanks a lot!

@IvyZX
Copy link
Collaborator

IvyZX commented Apr 10, 2023

Hey, can you fix the pre-commit hook errors? Then I can go ahead and merge it.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Neilblaze
Neilblaze commented Apr 11, 2023
View reviewed changes
Copy link
Contributor Author

@Neilblaze Neilblaze left a comment

Choose a reason for hiding this comment

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

@IvyZX PTAL :)

@IvyZX
Copy link
Collaborator

IvyZX commented Apr 17, 2023

Hi, could you rebase to the google/main branch and make sure all the tests pass? This is a large change and we don't want it to break head. Thank you!

@Neilblaze
Copy link
Contributor Author

@IvyZX Sure thing! I'll do it by EOD

@Neilblaze Neilblaze force-pushed the doc-patch-typo branch 2 times, most recently from 71190b7 to fb4d42b Compare April 20, 2023 21:54
@Neilblaze
fixed notebooks
823fd72 
Signed-off-by: Pratyay Banerjee <putubanerjee23@gmail.com>
@Neilblaze
Copy link
Contributor Author

@IvyZX done! ✅
image

@chiamp
Copy link
Collaborator

chiamp commented Apr 21, 2023

hi @Neilblaze, seems like there's a trailing whitespace error

@Neilblaze @cgarciae
fixed p-c hook trailing-whitespace
b48c765 
Co-Authored-By: Cristian Garcia <cgarcia.e88@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2023 •
edited
Loading

Codecov Report

Merging #3021 (b48c765) into main (332b7d0) will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3021      +/-   ##
==========================================
+ Coverage   81.97%   82.02%   +0.04%     
==========================================
  Files          55       55              
  Lines        6002     6035      +33     
==========================================
+ Hits         4920     4950      +30     
- Misses       1082     1085       +3
Impacted Files Coverage Δ
flax/core/lift.py 96.06% <ø> (ø)
flax/core/scope.py 89.80% <ø> (ø)
flax/errors.py 85.96% <ø> (ø)
flax/jax_utils.py 44.44% <ø> (ø)
flax/linen/linear.py 96.96% <ø> (ø)
flax/linen/transforms.py 94.14% <ø> (ø)
flax/metrics/tensorboard.py 92.85% <ø> (ø)
flax/serialization.py 94.56% <ø> (ø)
flax/training/common_utils.py 0.00% <ø> (ø)
flax/training/dynamic_scale.py 0.00% <ø> (ø)

... and 6 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@copybara-service copybara-service bot merged commit b3945f8 into google:main Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@8bitmp3 8bitmp3 8bitmp3 approved these changes

@IvyZX IvyZX IvyZX approved these changes

@cgarciae cgarciae cgarciae approved these changes

Assignees
No one assigned
Labels
pull ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Neilblaze @cgarciae @IvyZX @chiamp @codecov-commenter @8bitmp3