Skip to content

Conversation

@Bibo-Joshi
Copy link
Member

Makes all 3rd party dependencies optional, i.e. addresses https://github.com/python-telegram-bot/python-telegram-bot/projects/7#card-77767364.

Currently based against the read-only-cdc branch, b/c that must be merged first and the diff is better readible this way.

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
  • Documented code changes according to the CSI standard
  • Added myself alphabetically to AUTHORS.rst (optional)
  • Added new classes & modules to the docs and all suitable __all__ s

@Bibo-Joshi Bibo-Joshi added 📋 do-not-merge-yet work status: do-not-merge-yet 🛠 refactor change type: refactor 🛠 breaking change type: breaking ⚙️ dependencies affected functionality: dependencies labels Sep 30, 2022
@Bibo-Joshi Bibo-Joshi added this to the v20.0 milestone Sep 30, 2022
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! Looks like you edited the (optional) requirements or the pre-commit hooks. I'm just a friendly reminder to keep the additional dependencies for the hooks in sync with the requirements :)

@Bibo-Joshi Bibo-Joshi modified the milestones: v20.0, v20.0a5 Oct 6, 2022
Base automatically changed from read-only-cdc to master October 7, 2022 08:18
# Conflicts:
#	telegram/ext/_extbot.py
#	telegram/ext/_jobqueue.py
#	tests/test_callbackdatacache.py
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

LGTM

@harshil21 harshil21 removed the 📋 do-not-merge-yet work status: do-not-merge-yet label Oct 7, 2022
# Conflicts:
#	telegram/ext/_jobqueue.py
Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

Code changes look good to me.

I just realized we could make an optional version with 0 dependencies if we remove the httpx implementation for this version, allowing people to use their own httpx version. Might be nice so they can do whatever they want with no version conflicts.

@harshil21
Copy link
Member

@Poolitzer at that point there won't really be any difference between PTB and ptb-raw, I strongly think httpx should not be optional. I think having aps optional will also be quite annoying to most

@Poolitzer
Copy link
Member

@harshil21 yes, I think people will have to change their setup a lot.

@Bibo-Joshi
Copy link
Member Author

Two things that I forgot so far:

  • AppBuilder must no longer create a JobQueue by default
  • the timer bot example mist then be adjusted to tell AppBuilder to use a JQ
  • The docs of the conversation timeout parameter should mention that JQ and optional dependencies are needed

Copy link
Member

@Poolitzer Poolitzer left a comment

Choose a reason for hiding this comment

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

LGTM, great change

@Bibo-Joshi Bibo-Joshi added the ℹ️ needs-wiki-update information: needs-wiki-update label Oct 31, 2022
# Conflicts:
#	tests/test_bot.py
@Bibo-Joshi Bibo-Joshi merged commit e58cbcd into master Oct 31, 2022
@Bibo-Joshi Bibo-Joshi deleted the optional-deps branch October 31, 2022 09:12
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🛠 breaking change type: breaking ⚙️ dependencies affected functionality: dependencies ℹ️ needs-wiki-update information: needs-wiki-update 🛠 refactor change type: refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants