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

[Documentation] Add valuable information to attribute docstrings of telegram classes #3109

Closed
Bibo-Joshi opened this issue Jun 21, 2022 · 14 comments · Fixed by #3404
Closed

[Documentation] Add valuable information to attribute docstrings of telegram classes #3109

Bibo-Joshi opened this issue Jun 21, 2022 · 14 comments · Fixed by #3404
Labels
documentation good first issue

Comments

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Jun 21, 2022 •
edited by lemontree210

The docs of Telegram classes are currently written such that the docstring of the arguments are more informative than the docstring of the respective attributes. E.g. for Chat.has_private_forwards, "Returned only in get_chat()" is only present in the argument docstrings, not in the attribute docstring. For Message.text, the length limitation is only documented for the argument, not the attribute.

This logic makes sense for classes that are to be instantiated by the user (like BotCommand or InputMedia*), but for classes that are rarely instantiated by the user like Chat and Message this makes it sometimes hard to find valuable information.

I suggest to revisit the documentation of the Telegram classes and either unify the docstrings of args & attributes or at least check which information would be worth adding to the attributes as well.

Contributions in this direction are very welcome! They also don't have to cover all Telegram classes at once - instead this can be done step by step. If you want to cover (parts of) this, please leave a comment below, so that we can avoid duplicate work.

PRs for this should be made against the doc-fixes branch. Please also check out the contribution guide, especially the part on how to build the documentation locally.

Classes are listed below for convenience.
@Bibo-Joshi Bibo-Joshi mentioned this issue Jun 21, 2022
Closed
24 tasks
@Tosin-JD
Copy link

I did like to participate in contribute in making the above changes as a beginner in open source.

@Bibo-Joshi
Copy link
Member Author

You're very welcome to contribute! please leave a short comment about which classes you want to cover in a pull request :)

@Bibo-Joshi Bibo-Joshi mentioned this issue Jun 26, 2022
Closed
@simonfongnt
Copy link
Contributor

is this still available for a beginner?

@Bibo-Joshi
Copy link
Member Author

You're very welcome to contribute! There haven't been any PRs regarding this so far. please leave a short comment about which classes you want to cover in a pull request :)

@simonfongnt
Copy link
Contributor

i shall go for the Message class and unify the docstrings of args & attributes in the PR.

@simonfongnt simonfongnt mentioned this issue Aug 30, 2022
Merged
5 tasks
This was referenced Oct 1, 2022
Merged
Closed
Closed
@penguinpee
Copy link
Contributor

Collision avoidance: I'll start at the bottom and take care of the _web family of classes.

Happy Hacktober! 🍻

@Bibo-Joshi
Copy link
Member Author

Thanks for the notice @penguinpee . But can you clarify which classes exactly you mean? There are no classes in the telegram package whose name ends on _web or Web 😄

@penguinpee
Copy link
Contributor

I was referring to the classes in telegram/_web*.py. The corresponding classes are called Web*.

penguinpee added a commit to penguinpee/python-telegram-bot that referenced this issue Oct 12, 2022
@penguinpee
Unified docstrings
a263163 
- copied mostly from args to attributes
- fixed language
- fixed and improved formatting
- contribution to python-telegram-bot#3109
@penguinpee penguinpee mentioned this issue Oct 12, 2022
Merged
@penguinpee
Copy link
Contributor

I'm willing to do more to make this a worthwhile Hacktober contribution.

This was referenced Oct 22, 2022
Closed
Merged
@HatimZ
Copy link
Contributor

HatimZ commented Oct 22, 2022

I did the Chat class and unified the docstrings of args & attributes in the PR.

@lemontree210 lemontree210 mentioned this issue Oct 25, 2022
Merged
@RobiMez
Copy link
Contributor

RobiMez commented Oct 26, 2022

Tackling the poll class for a start.

@RobiMez
Copy link
Contributor

RobiMez commented Oct 26, 2022

#3312

@RobiMez
Copy link
Contributor

RobiMez commented Oct 28, 2022

#3314

@HatimZ HatimZ mentioned this issue Oct 28, 2022
Closed
@lemontree210
Copy link
Member

lemontree210 commented Dec 1, 2022 •
edited

List of classes: ("NCN" = "No changes needed")

@lemontree210 lemontree210 mentioned this issue Dec 2, 2022
Merged
@lemontree210 lemontree210 linked a pull request Dec 16, 2022 that will close this issue
Unified args and attrs in classes #3404
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
documentation good first issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Unified args and attrs in classes python-telegram-bot/python-telegram-bot
8 participants
@Bibo-Joshi @harshil21 @simonfongnt @RobiMez @Tosin-JD @lemontree210 @HatimZ @penguinpee