-
Notifications
You must be signed in to change notification settings - Fork 5.7k
[Feature] Improve handling of local mode & docs regarding file handling #3114
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
Comments
I think it is a small improvement, and I don't think that much more benefits will come to the local API, especially not some which would change further how our library handles stuff. On the other hand, its a big change (and allows the local server to be used as intended), so I think its a good one. I think a note for every input handling a file is the best solution, and since that would take quite some additional screen time, linking to a second page where it is explained in detail how file handling works in PTB is good. |
I'm a big confused here 😄 "I think it is a small improvement" vs "its a big change". Do you mean that the added value is small while the code/compatiblity change is big? |
Sorry, mistyped. Small change, big improvement. |
About the sphinx issue linked.. I found an alternative - We could use rst Substitutions in such places too. Maybe not as effective as an include, but it would reduce repetition in
class Bot(TGObject):
"""....
.. |read_timeout| replace:: Value to pass to ...
"""
async def get_me(...):
"""
Keyword Args:
read_timeout (:obj:`....`): |read_timeout|
...
""" If you all agree this is effective enough for the time being, I can open a good first issue for this. |
Interesting! covering the name & type part probably doesn't work? if not: TBH I feel like it would be worth to first try and PR to sphinx about sphinx-doc/sphinx#9939. OTOH, switching from a |
I suggest to add a flag
local_api_server
/local_mode
/similar naming to theBot
class which can be used to tell ptb that the bot is running against a bot api server in--local
mode. Currently the benefit of that would be that we could accept file paths in all cases. In local mode, we pass the URI, otherwise we load the contents in binary mode.If the local mode has more benefits in the future, we could leveral this setting.
What do the other team members think about this?
Additionally, we should unify the documentation on how files are handled.
Some of the classes/methods that accept file input have a note like here, but others don't. Also that note is outdated, as it doesn't explain when a
pathlib.Path
can be passed. It should also mention that we don't supportopen(file, 'r')
but onlyopen(file, 'rb')
. We should either have a unified note that we add everywhere or move this snippets section to a standalone page, extend it a bit and point to it from the docs.I'm slightly in favor of the latter.
To avoid repitition of
Note: …
strings in the docs, we can.. include::
them - see sphinx-doc/sphinx/issues/9939The text was updated successfully, but these errors were encountered: