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

Docs cross references (#2633) #2855

Closed
wants to merge 50 commits into from
Closed

Docs cross references (#2633) #2855

wants to merge 50 commits into from

Conversation

pavel-ignatovich
Copy link

At the moment, only the first part of the cross-referencing task has been completed, related to links within the source code, excluding the wiki and examples.

Checklist for PRs

  • Added .. versionadded:: version, .. versionchanged:: version or .. deprecated:: version to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
  • Created new or adapted existing unit tests
  • Added myself alphabetically to AUTHORS.rst (optional)
  • Added new classes & modules to the docs

Bibo-Joshi and others added 30 commits January 3, 2022 09:42
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Signed-off-by: starry69 <starry369126@outlook.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: poolitzer <25934244+Poolitzer@users.noreply.github.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hey! Looks like you edited the (dev) requirements or the pre-commit hooks. I'm just a friendly reminder to keep the pre-commit hook versions in sync with the dev requirements and the additional dependencies for the hooks in sync with the requirements :)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hey! Looks like you edited README.rst or README_RAW.rst. I'm just a friendly reminder to apply relevant changes to both of those files :)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)

@Bibo-Joshi Bibo-Joshi mentioned this pull request Jan 11, 2022
24 tasks
Copy link
Member

@Bibo-Joshi Bibo-Joshi 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 your PR - nice work! Concentrating on inter-doc linking is perfectly fine 👍
I left a few comments below. Apart from that, I have two more general comments:

  • it would be nice to have some more cross-referencing in tg.ext. e.g. link from Handler to Dispatcher.add/remove_handler, from CallbackContext.job to JobQueue, from {CallbackContext, Dispatcher.{chat, bot, user}_data} to ContextTypes, from ContextTypes to {Updater,Dispatcher}Builder.context_types, from Defaults to {Updater, Dispatcher}Builder.defaults and ExtBot.defaults, from CallbackDataCache to ExtBot.callback_data_cache and CallbackContext.drop_callback_data, from Dispatcher.update_persistence to BasePersistecne.update_*_data. You can probably find some more :) That said, I'm ofc aware, that it's probably impossible to find all the links :D
  • Having each reference on a new line seems a bit excessive and uses a lot of space. Can you remove the empty lines and add commas at the end of the lines? that way we get a more compact overview

@@ -2345,6 +2460,12 @@ def get_file(
You should save the file's MIME type and name (if available) when the File object
is received.

.. seealso:: :meth:`telegram.ChatPhoto.get_small_file`
Copy link
Member

Choose a reason for hiding this comment

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

here, we also have {Animation, Audio, Document, PhotoSize, Sticker, Video, VideoNote, Voice}.get_file. Please also check that you link back to here from those methods :)

@@ -42,6 +42,10 @@ class ChatMember(TelegramObject):
Objects of this class are comparable in terms of equality. Two objects of this class are
considered equal, if their :attr:`user` and :attr:`status` are equal.

.. seealso:: :attr:`telegram.ChatMemberUpdated.old_chat_member`
Copy link
Member

Choose a reason for hiding this comment

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

also tg.ChatMember, Bot.get_chat_member, Bot.get_chat_administrators

@@ -36,6 +36,8 @@ class ChatPermissions(TelegramObject):
permissions that are set, but also sets all the others to :obj:`False`. However, since not
documented, this behaviour may change unbeknown to PTB.

.. seealso:: :attr:`telegram.Chat.permissions`
Copy link
Member

Choose a reason for hiding this comment

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

and Bot.set_chat_permissions

@@ -34,6 +34,8 @@ class ChatPhoto(TelegramObject):
considered equal, if their :attr:`small_file_unique_id` and :attr:`big_file_unique_id` are
equal.

.. seealso:: :attr:`telegram.Chat.ChatPhoto`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. seealso:: :attr:`telegram.Chat.ChatPhoto`
.. seealso:: :attr:`telegram.Chat.chat_photo`

Comment on lines +49 to +52
.. seealso:: :attr:`telegram.CallbackQuery.media`

:attr:`telegram.messages.media`

Copy link
Member

Choose a reason for hiding this comment

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

InputMedia objects are never sent back from telegram, they are only used as input for send_media_group or edit_message_media. Please also add this to the subclasses - btw same for ChatMember :)

@@ -39,6 +39,7 @@ class KeyboardButton(TelegramObject):
* :attr:`request_poll` option will only work in Telegram versions released after 23
January, 2020. Older clients will receive unsupported message.

.. seealso:: :attr:`telegram.ReplyKeyboardMarkup.keyboard`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. seealso:: :attr:`telegram.ReplyKeyboardMarkup.keyboard`
.. seealso:: :attr:`telegram.ReplyKeyboardMarkup.keyboard`

@@ -84,6 +84,22 @@ class Message(TelegramObject):
Note:
In Python :keyword:`from` is a reserved word, use ``from_user`` instead.

.. seealso:: :meth:`telegram.Bot.send_message`
Copy link
Member

Choose a reason for hiding this comment

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

all the other send_* methods also return Messages and so do the edit_* methods … IMO that's in fact a bit much cross referencing - IMO we can just skip all of those here, including send_message

@@ -96,6 +98,10 @@ class BasePersistence(Generic[UD, CD, BD], ABC):
.. versionchanged:: 14.0
The parameters and attributes ``store_*_data`` were replaced by :attr:`store_data`.

.. seealso:: :attr:`telegram.ext.ConversationHandler.persistence`

:attr:`telegram.ext.Dispatcher.persistence`
Copy link
Member

Choose a reason for hiding this comment

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

{Updater, Dispatcher}Builder.persistence

@@ -503,6 +503,8 @@ class DispatcherBuilder(_BaseBuilder[ODT, BT, CCT, UD, CD, BD, JQ, PT]):
.. seealso::
:class:`telegram.ext.UpdaterBuilder`

:meth:`telegram.ext.Dispatcher.builder`
Copy link
Member

Choose a reason for hiding this comment

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

this doesn' make much sense IMO, please remove. same below.

@harshil21
Copy link
Member

Wow, do we really need so many cross references? Can't imagine adding/maintaining each one of these after refactors/API updates

Comment on lines +105 to +107
:attr:`telegram.InlineQuery.from_user`

:attr:`telegram.InlineQuery.from_user`
Copy link
Member

Choose a reason for hiding this comment

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

repeated

Comment on lines +32 to +34
.. seealso:: :attr:`telegram.InputInvoiceMessageContent.prices`

.. seealso:: :attr:`telegram.ShippingOption.prices`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. seealso:: :attr:`telegram.InputInvoiceMessageContent.prices`
.. seealso:: :attr:`telegram.ShippingOption.prices`
.. seealso:: :attr:`telegram.InputInvoiceMessageContent.prices`,
:attr:`telegram.ShippingOption.prices`

@harshil21 harshil21 added this to the v14 milestone Jan 12, 2022
@Bibo-Joshi
Copy link
Member

Wow, do we really need so many cross references? Can't imagine adding/maintaining each one of these after refactors/API updates

Some are surely more useful than others. E.g. Linking from ReplyMarkup to Message.reply_markup etc has surely lesser value than linking from Bot.send_message to Message.reply_text & friends. But having it also doesn't hurt.
And especially for the pure API part I'd be okay if we don't catch everything always.

@pavel-ignatovich
Copy link
Author

pavel-ignatovich commented Jan 12, 2022

Thanks for feedback! I will be working on fixes of issues listed in comments in the coming days =)

@Bibo-Joshi Bibo-Joshi deleted the branch python-telegram-bot:doc-fixes February 9, 2022 16:30
@Bibo-Joshi Bibo-Joshi closed this Feb 9, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet