-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Clear up imports policy #2468
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 don't like 1, because that would mean loosing all the tidiness we have right now, or at least the attempt for this. I would really like 2 though, we should make a new class for the helpers imo, and provide there all the methods we intend for users from the helpers :D |
With Noam also in favor of 2, I'll add the 💥 tag and postpone to v14 |
Random thought, maybe we can do like |
Interesting ideas! In fact "project name = name of the only package provided by the project" is strongly encouraged by PEP 423. If the goal ist just to have the Regarding structure: In your proposed solution, would In any case, even if we decide to continue discussion in this direction, I guess we can still first go ahead with the changes mentioned above. Renaming/ordering the packages can still be done an the end of v14 phase 1 or after phase 2. |
Urgh, yeah, no underscores then
Agree on the too much trouble with renaming anything but the import. I would stick with something close to python_telegram_bot, I like underscores
But that's exactly the issue imo. People see import telegram and think "well, I will do a
The main import will only contain subdirectories/modules/however you want to call them. And I do think it is worth the clarity, yes.
Hm, the whole idea was adding a basename? Not sure I get the but part. And we can either add telegram/ext specific util modules, or maybe we do basename.utils.telegram and basename.utils.ext.
Sure. |
you're right, that's a point I missed. I guess this is also the reason for this
and next line
Whaaaaat? :P I do see how |
Uh, I made an upsi there. I wanted to suggest Its long, but you only import once. |
Meh … We should probably get opinions from the others on this before we get lost in discussion possible names :D
once per file, though |
We should be able to get the |
Totally forgot that this is possible :D Question is, what would we do if we had access to that pypi project?
|
@Bibo-Joshi Pretty sure harshil meant PTB, not telegram |
Hmm, good points. How about this: We add this requirement in requirements.txt: And iirc, ever since |
@harshil21 Doesn't stop them from installing telegram and complaining it wont work, but maybe eliminates the namespace issue, jup. |
We're currently not very consistent regarding what we make available directly via
telegram.__init__
and what we don't. Some examples:telegram.error
onlyTelegramError
is available viatelegram.__init__
, but no of the other classestelegram.constants
only some few constants are available.TelegramDeprecationWarning
is not available (see also Overhaul of warnings #2467)utils.helper
that are actually there to be used by users and not exclusively by PTB itself (e.g.escape_markdown
,mention_user_*
) are not availableIMO we should try to make a clear policy for this. IMHO it should be clear that all API-classes are directly available through
telgeram
. All the things mentioned above are from … lets call them "auxiliary modules". For these I basically see two options:telegram
telegram
. In this case I'd vote to at least move the above mentioned helper methods directly totelegram.utils
and spare the user the.helpers
, especially as users are in general not to be concerned with the other contents ofutils
.Case 2 would be breaking & in that case we should wait until v14.
In any case I would aim to document the import policy somewhere … probably just at the index of the docs.
I'm slightly in favor of 2. What do the other @python-telegram-bot/developers think?
The text was updated successfully, but these errors were encountered: