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

Documentation links in tutorials #136

Merged
merged 26 commits into from
Oct 2, 2023
Merged

Conversation

pseusys
Copy link
Collaborator

@pseusys pseusys commented May 27, 2023

Description

Added some documentation links to some of the tutorials.

Checklist

  • I have covered the code with tests
  • I have added comments to my code to help others understand it
  • I have updated the documentation to reflect the changes
  • I have performed a self-review of the changes

@pseusys pseusys requested review from kudep and RLKRo May 27, 2023 00:56
@pseusys pseusys marked this pull request as ready for review June 16, 2023 03:26
@pseusys pseusys self-assigned this Jun 16, 2023
@pseusys pseusys added the documentation Improvements or additions to documentation label Jun 16, 2023
@RLKRo
Copy link
Member

RLKRo commented Jul 5, 2023

Maybe use local referencing? That way it'd be possible to view the links even if they are not on github.

Instead of

[module](https://deeppavlov.github.io/dialog_flow_framework/apiref/dff.context_storages.database.html)
[object](https://deeppavlov.github.io/dialog_flow_framework/apiref/dff.context_storages.database.html#dff.context_storages.database.context_storage_factory)

use

[module](../apiref/dff.context_storages.database.html)
[object](../apiref/dff.context_storages.database.html#dff.context_storages.database.context_storage_factory)

@RLKRo
Copy link
Member

RLKRo commented Jul 19, 2023

Also, would it be possible to create a function that, like insert_installation_cell_into_py_tutorial, modifies source code of tutorials, but for creating documentation links?
So that we could write something like

[module](!link(context_storages.database))
[object](!link(context_storages.database,context_storage_factory))

and it would be replaced with

[module](../apiref/dff.context_storages.database.html)
[object](../apiref/dff.context_storages.database.html#dff.context_storages.database.context_storage_factory)

EDIT:
We could also use the same approach for dependency installation cells.

This would allow us to specify to which dependencies are required for each tutorial or to omit the installation cell:
So a string like

# !install dff[ydb], streamlit

will generate an installation cell with !python3 -m pip install -q dff[ydb], streamlit.

And if there is no such string, the installation cell will not be present in the file.

@kudep kudep marked this pull request as draft July 26, 2023 09:06
@kudep
Copy link
Collaborator

kudep commented Jul 26, 2023

We add template for macros in another PR, now fix linking

@pseusys pseusys marked this pull request as ready for review July 26, 2023 14:14
Copy link
Member

@RLKRo RLKRo left a comment

Choose a reason for hiding this comment

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

Is the doc building successfully for you?

For me, the first cell is not building
image

I am able to fix that by removing # noqa: E501 from the cells.

Obviously, that will fail our linter.

One possible solution would be to reformat the way we insert links:

instead of

Here, [PollingTelegramInterface](https://deeppavlov.github.io/dialog_flow_framework/apiref/dff.messengers.common.interface.html#dff.messengers.common.interface.PollingMessengerInterface)
class and [telebot](https://pytba.readthedocs.io/en/latest/index.html) library are used for accessing telegram API in polling mode.

use

Here, [PollingTelegramInterface](
../apiref/dff.messengers.common.interface.html#dff.messengers.common.interface.PollingMessengerInterface
) class and [telebot](
https://pytba.readthedocs.io/en/latest/index.html
) library are used for accessing telegram API in polling mode.

Unfortunately, that is still 105 characters.

Seems like the easiest solution is to wait for an implementation of the aforementioned tutorial patterns.

tutorials/context_storages/1_basics.py Outdated Show resolved Hide resolved
tutorials/context_storages/2_postgresql.py Outdated Show resolved Hide resolved
tutorials/context_storages/2_postgresql.py Outdated Show resolved Hide resolved
docs/source/utils/notebook.py Outdated Show resolved Hide resolved
tutorials/messengers/telegram/1_basic.py Outdated Show resolved Hide resolved
tutorials/script/core/9_pre_transitions_processing.py Outdated Show resolved Hide resolved
tutorials/script/responses/2_buttons.py Outdated Show resolved Hide resolved
tutorials/script/responses/3_media.py Outdated Show resolved Hide resolved
tutorials/utils/1_cache.py Outdated Show resolved Hide resolved
docs/source/utils/notebook.py Outdated Show resolved Hide resolved
@pseusys pseusys requested a review from RLKRo August 3, 2023 11:16
@pseusys pseusys marked this pull request as draft August 3, 2023 11:16
@pseusys pseusys mentioned this pull request Aug 14, 2023
4 tasks
@pseusys pseusys marked this pull request as ready for review August 21, 2023 08:16
@pseusys pseusys requested a review from kudep August 21, 2023 08:16
@pseusys pseusys mentioned this pull request Aug 21, 2023
4 tasks
Copy link
Member

@RLKRo RLKRo left a comment

Choose a reason for hiding this comment

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

I really like mddoclink. Makes it difficult to insert an incorrect link:
image

docs/source/utils/notebook.py Outdated Show resolved Hide resolved
docs/source/utils/notebook.py Outdated Show resolved Hide resolved
docs/source/utils/notebook.py Outdated Show resolved Hide resolved
docs/source/utils/notebook.py Show resolved Hide resolved
docs/source/utils/notebook.py Outdated Show resolved Hide resolved
tutorials/context_storages/2_postgresql.py Outdated Show resolved Hide resolved
tutorials/messengers/web_api_interface/1_fastapi.py Outdated Show resolved Hide resolved
tutorials/utils/1_cache.py Outdated Show resolved Hide resolved
@pseusys
Copy link
Collaborator Author

pseusys commented Aug 24, 2023

Waiting for #144

@pseusys pseusys requested a review from RLKRo September 30, 2023 21:53
@RLKRo RLKRo merged commit 56af13d into dev Oct 2, 2023
16 checks passed
@RLKRo RLKRo deleted the feat/documentation_links_in_tutorials branch October 2, 2023 19:12
@RLKRo RLKRo mentioned this pull request Oct 12, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants