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

Fix typo #1030

Merged
merged 4 commits into from
Aug 14, 2024
Merged

Fix typo #1030

merged 4 commits into from
Aug 14, 2024

Conversation

gil2rok
Copy link
Contributor

@gil2rok gil2rok commented Jul 30, 2024

Change scheduels to schedules. Also remove unnecessary comma.

Change scheduels to schedules. Also remove unnecessary comma.
Copy link
Collaborator

@vroulet vroulet left a comment

Choose a reason for hiding this comment

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

Thanks!

@gil2rok
Copy link
Contributor Author

gil2rok commented Jul 30, 2024

Would love some advice on small doc improvements for the GradientTransformation class in base.py.

  1. The doc refers to Optax's meta-learning example. I added a link to this in the doc: https://optax.readthedocs.io/en/latest/_collections/examples/meta_learning.html. Would a relative path to the meta-learning example be preferred? If so, how is this formatted?
  2. If the link format is preferred, it goes past the end of line that the rest of the doc respects. How are long links handled?
  3. There are a couple weirdly formatted underscores. Not sure what they are trying to accomplish, but wanted to highlight this:
Screenshot 2024-07-30 at 3 54 25 PM

@vroulet
Copy link
Collaborator

vroulet commented Jul 31, 2024

Hello @gil2rok,

  1. The docs use reStructuredText whose semantics are not the same as usual markdown. See e.g. https://docs.anaconda.com/restructuredtext for some tips. In this case, I would simply use
`meta-learning example <https://optax.readthedocs.io/en/latest/_collections/examples/meta_learning.html>`_

There may be a way to link internal refs but that's not very important.
2. You may add

# pylint: disable=line-too-long
(The doc string...)
# pyline: enable=line-too-long

to avoid linting issues because of too long lines. (see e.g. rmsprop in the alias file)
3. I suppose the intent was to emphasize these elements if the docs were in markdown. But as said above the docs use reStructuredText (rst) so one should use the syntax of rst to emphasize things here. (Or we may simply remove these underscores, it's not super important as long as it is displayed in a readible fashion which is not the case right now).

Thanks again!

@gil2rok
Copy link
Contributor Author

gil2rok commented Aug 1, 2024

Silly mistake. I didn't realize docs are in rst and not in Markdown!

All fixed now.

@vroulet
Copy link
Collaborator

vroulet commented Aug 1, 2024

Thanks again, can you send a screenshot of the resulting doc?
Also as you said you may replace the underscores _ by the corresponding emphasizing syntax in rst (wrap text between asterisks *).

@gil2rok
Copy link
Contributor Author

gil2rok commented Aug 2, 2024

All done!

Screenshot 2024-08-01 at 10 43 05 PM

@copybara-service copybara-service bot merged commit 8273d72 into google-deepmind:main Aug 14, 2024
8 checks passed
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