Skip to content

Overhaul of Filters #2759

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

Merged
merged 69 commits into from
Nov 20, 2021
Merged

Overhaul of Filters #2759

merged 69 commits into from
Nov 20, 2021

Conversation

harshil21
Copy link
Member

@harshil21 harshil21 commented Oct 30, 2021

Breaking changes:

Also fixes #2281 (the problem was that we had duplicate docs in filters.)

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) How do we want to document this breaking change? Just a .. versionchanged:: on top of the filters module?
  • Created new or adapted existing unit tests

Notable changes:

  • Converted __new__ in BaseFilter to __init__ see: Overhaul of Filters #2759 (comment)

  • Made _UpdateType public, so users can now do: filters.UpdateType.MESSAGES.

  • deleted ChatType's filter() as discussed offline (reasoning was that Filters.chat_type was always True) .

  • All _ChatUserBaseFilter subclasses have a CAPS shortcut with allow_empty=True, we can discuss to remove this / keep it default (False). This allows a user to do something like filters.SENDER_CHAT work for both Channels and Supergroups.

    Edit: It was decided that those shortcuts won't inherit from _CUBF and instead be their own, see
    https://t.me/pythontelegrambotdev/261 for discussion.

  • Use a functools.partial() in DICE filter to simplify it for the user. See the examples section in Dice docs.
    Reverted. See: Overhaul of Filters #2759 (comment)

Minor changes:

  • remove the name argument in _Dice (to reduce repetition, it is now built from the emoji itself)
  • changed import of User, Chat in filters to TG{User, Chat} to avoid namespace conflicts.
  • The filters docs are now arranged according to how it appears in source.
  • Converted Optional[str] -> str in MimeType and Category (it was just plain wrong).
  • Rearrange filters in the source (right now they're all over the place). Should be done after a approving review since the diff is already messy.

Bibo-Joshi and others added 30 commits October 3, 2021 20:12
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Signed-off-by: starry69 <starry369126@outlook.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: poolitzer <25934244+Poolitzer@users.noreply.github.com>
Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
@harshil21 harshil21 removed the 📋 pending-review work status: pending-review label Nov 2, 2021
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

nice updates! I left a number of more comments though I'd classify most of them as nitpicking by now 😃

Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

One detail, otherwise I'm happy :)
IIRC you'll sort the contents of filters.py now, right?

I'll force-push v14 to include the master changes later tonight.
If you want to, this could be a good chance to get to know rebasing :D basically, you (pull v14 and) run git rebase v14 and follow the comments in the command line. I guess there should be few to none merge conflicts :)
Alternatively, just merging v14 into this branch is fine as well ofc :)

PS: this article helped me quite well when working with rebase the first time

@harshil21 harshil21 merged commit 4698bc4 into v14 Nov 20, 2021
@harshil21 harshil21 deleted the refactor-filters branch November 20, 2021 10:36
harshil21 added a commit that referenced this pull request Nov 20, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2021
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🛠 breaking change type: breaking 🔌 enhancement pr description: enhancement 🛠 refactor change type: refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Sphinx warnings Refactor Filters
10 participants