Hey, I'm new to open source and want to learn how to contribute, and I can't quite get what needs to be done here. Can you guide me on which doc is required in order to be updated or changed?

Hey, nice to hear.

So I already linked to a few examples in my original message. Have a look at the source doc string of send_poll. As you can see in question, it links the 1-number to :tg-const:`telegram.Poll.MAX_QUESTION_LENGTH` instead of writing out the number in the doc string itself (this happens five arguments later in explanation, where it says 0-200 characters without the link).

If you have a look at that linking, you can see that it references MAX_QUESTION_LENGTH in the Poll file. In the source of Poll, you can see that you have been bamboozled and the actual source for these variables lies in the constants.py file, MAX_QUESTION_LENGTH is just a link to constants.PollLimit.QUESTION_LENGTH.

In the source of constants, you can finally see how it is implemented: An IntEnum Class which has the values mapped to their respective key.

The issue is that we only did this sparingly thus far, whenever we thought of it. I would like to replace every limit currently written out in the docstrings with links to constants files. We as maintainer still have decide on a couple things.

Now there is the question of what you can do in the meantime. What would be an amazing help if you can go through the whole documentation (maybe scroll through telegrams one, its easier to read one after another since its just one page). Any missed limitation (last point in the list below) would be hugely appreciated pointed out, since that would require a decision from us before one can implement it.

Otherwise you could go ahead and convert some obvious limitations already.

On a side-note, I will push an example improvements of the invoice object limitations in the near future, I will ping this issue to it, so one can see a clear example of before>after.

Maintainers have to decide on:

• Include Min and Max limits of a given constraint in the constants?
• I would say yes definitely, but in the example linked above we only did max for some reason, and in quite some other places as well.
• Include type restrictions (e.g. only latin characters for deep links). If yes, how?
• I vote yes again, I think this makes sense as a thing. I am not sure about the implementation, I would provide a function for it I guess which takes a string as input and yields false or true if everything is allowed or not. Maybe a better return which character was wrong?
• Include two/three-letter ISO code restriction
• I don't know about this. Neither yes nor no for now.
• Something else I missed.
• Have I?

• Include Min and Max limits of a given constraint in the constants?

I would say yes for the Min only if it's non zero, e.g. in sendPoll.open_period and switch_pm_parameter that would make sense. But not in for e.g. answer_callback_query.text

• Include type restrictions (e.g. only latin characters for deep links). If yes, how?

I don't really understand this. Do you want to enforce this at runtime? I thought we decided against that in #2901?

• Include two/three-letter ISO code restriction

Also no, see above linked issue + I think they can change over time so that's just added maintenance for us.

I don't really understand this. Do you want to enforce this at runtime? I thought we decided against that in #2901?

No nothing here is enforcing at runtime, its a question of how we interpret entries to our constant files. On one side we have the usual API constants, on the others we have the max/min limits of specific calls/objects, so people can do if length(input) > Poll.MaxTitle: do_error.

The last two entries in the list are continuations of the second type. With the second they could do if not constants.SwitchPMValidation(input): do_error, with the third if not two/three_letter_ISO(input): do_error.

I think they can change over time

They have never. But thats the same for any other entry to constants.

Ah okay. I guess having those validation functions for "constants" is not very fitting, since it's meant to be hardcoded values and nothing else? Maybe having them in a separate validators.py file as functions is better? Are there other places where we can validate input?

What would be an amazing help if you can go through the whole documentation

Or just regex-search for integers 🙂

• Include Min and Max limits of a given constraint in the constants?

I would say yes for the Min only if it's non zero

+1

• Include type restrictions (e.g. only latin characters for deep links). If yes, how?

I guess having those validation functions for "constants" is not very fitting, since it's meant to be hardcoded values and nothing else? Maybe having them in a separate validators.py file as functions is better? Are there other places where we can validate input?

I agree with harshil here. constants is for constants, not validators. Personally I see the main advantage of the constants module in not having to hard code values/define those constants on your own. How a user uses them is another question.
Validators could go into helpers, though personally I don't see a big need to do that. If we get feature requests for that - sure. ptbcontrib is another possibility.

• Include two/three-letter ISO code restriction

Punshline the same as above, with the addition that a maintained list of ISO-language codes very likely already exists somewhere and I don't see a need to duplicate that in PTB.

Alright, lets drop validation stuff for now and let these limits in the doc strings, we can stick to the other hardcoded values. One example on how this is implemented is visual in #3112:

The doc strings of InputInvoiceMessageContent and bot.send_invoice changed, the constants were linked in Invoice and finally implemented in InvoiceLimits enum in the constants file.

Hope this makes clear how we expect this issue to be implemented.

I kind of like getting rid of magical numbers so I could dive into this unless you'd like me to focus on something more pressing

How about default values? My guess is that we do not put them into the enumerations because default values (if we are talking about Telegram API, not additional kwargs) are not passed to Telegram: None is passed and then Telegram applies the default value, as opposed to PTB passing the default value.

That would mean that, for example, for parameter max_connections of Bot.set_webhook() I will add constants for min (1) and max (100), but not for default value (40), which will remain just a number 40 (without a hyperlink) in the docstring.

Even if I added a constant for that too, that wouldn't mean a change in the signature of set_webhook(): max_connections would still default to None.

The example for Bot.send_invoice() had no such cases (non-zero defaults), so I just wanted to check that understanding.

@lemontree210 yes, that understanding is correct. No need to make constants for default values.

One example on how this is implemented is visual in #3112:

The doc strings of InputInvoiceMessageContent and bot.send_invoice changed, the constants were linked in Invoice and finally implemented in InvoiceLimits enum in the constants file.

The files actually demonstrate two approaches: class Invoice contains class constants that are linked to InvoiceLimits enum, but class Bot shows a different approach to WebhookLimit. Bot's docstring contains direct references to WebhookLimit in constants.py, no class constants were declared in Bot itself.

Which approach should I follow? I assume the former (like in Invoice), but I wanted to double check. Also, if the approach for InvoiceLimit is the best, then maybe I should also add WebhookLimit's constants to Bot.

Which approach should I follow? I assume the former (like in Invoice), but I wanted to double check.

I don't think we should add class constants for Bot, since Bot contains many methods and most of them link to different constants.