Add file extension filter

[eIGato]

Resolve #2140.
Based on an abandoned PR of another contributor.
Fix #2156.

[eIGato]

@Bibo-Joshi may you restart failed tests? I don't think they failed because of these changes. More likely, some temporary issue with test env.
Besides, no need to edit keywords in this PR. "Resolve" and "Fix" are fine too.

[eIGato]

Thanks.

[Bibo-Joshi]

NP. I'll try to review both PRs tomorrow ;)

[eIGato]

I think everything is fixed here.

[Bibo-Joshi]

As a side note: Please don't force push if not necessary. It makes reviewing harder.

[eIGato]

What about case sensitivity? I've made it insensitive but may be there are cases when it should not be done. For example *.Dockerfile. May be make it optional or just score on this?

[eIGato]

Sorry for force-pushing. I'm used to Gitlab where one can view diffs of different pushed versions. I just want to bring logically connected changes to one commit.

[Bibo-Joshi]

What about case sensitivity? I've made it insensitive but may be there are cases when it should not be done. For example *.Dockerfile. May be make it optional or just score on this?

Mh, good point. While intuitively I'd say lower case everything, a quick search tells me that file extension case sensitivity can depend on for operating system and also hard drive encoding or something. So I'd like to go for a flag, defaulting to lowercasing everythng. have a look at the implementation of Filters.command for that. Please add something like

Note:
  * adds the dot automatically, so *insert .jpg example here*
  * case insensitive by default, use the flag *whatever it's called*, if you need it

to the docstring

Sorry for force-pushing. I'm used to Gitlab where one can view diffs of different pushed versions. I just want to bring logically connected changes to one commit.

NP. We squash anyway before merging, so that doesn't really matter too much ;)

[Bibo-Joshi]

Thanks for the updates! Just two nitpicks left from my side. @Poolitzer do you want to review, too?

[eIGato]

Cite from your contibuting guidelines: "This gives us the flexibility to re-order arguments and more importantly to add new required arguments".
Why should it refer only to calls but not the signatures?

[Bibo-Joshi]

Cite from your contibuting guidelines: "This gives us the flexibility to re-order arguments and more importantly to add new required arguments".
Why should it refer only to calls but not the signatures?

Well, that style guideline applies only to code within PTB itself and not to how users should use the library. So not having , *, in the signatures allows the user to use both.
I do see the advantage of enforcing kwargs in calls, but I also see the philosophy of not handholding the user and the fact that it's a breaking change that should be done consistently throughout the lib.
If it's important to you, please go ahead and open a new issue so that it can be properly discussed, as it's beyond the scope fo this PR.

[eIGato]

It's a normal approach: use a good practice for the new code and fix the old code later.
Did as you wish. Any nitpicks left?

[Bibo-Joshi]

not from my side, but Poolitzers review is still pending. Same for #2167 btw.

[Poolitzer]

Left some minor comments, overall I really like this PR, thanks!

[Poolitzer]

sorry what is the !r for?

[Bibo-Joshi]

it adds quotes to the strings. Also make None distinguishable from "None". Also asked that :D

[Poolitzer]

quick google didnt show that to me :(

[Bibo-Joshi]

uses repr() instead of str()

[Poolitzer]

oh @Bibo-Joshi can you check if you can build docs on this branch, specifically the filters page and this filter appears?

[Bibo-Joshi]

oh @Bibo-Joshi can you check if you can build docs on this branch, specifically the filters page and this filter appears?

It doesn't. @eIGato the docstring of file_extension needs to be copied into the docstring of document. Please do that and build the docs locally to check that it renders correctly.
thanks for the catch @Poolitzer !

[eIGato]

IDKY the checks fail. New code is fully covered with tests and all tests pass locally: 1716 passed, 187 skipped, 19 xfailed, 1 xpassed in 207.14 seconds.
I've tried to repush the branch to restart the checks but result did not change.

[Bibo-Joshi]

test official fails due to the recent API updates not yet being incorporated and codecov for some reason decided to mark a docstring as not tested, so everthing's fine.

[eIGato]

IDK how to fix such rendering: the renderer just understands the line feeds literally.

[Bibo-Joshi]

I fixed the rendering locally, but I can't push to your branch. did you disable edits by maintainers (very bottom of the panel on the right)? if not:

        file_extension: This filter filters documents by their file ending/extension.

            Note:
                * This Filter only filters by the file ending/extension of the document,
                  it doesn't check the validity of document.
                * The user can manipulate the file extension of a document and
                  send media with wrong types that don't fit to this handler.
                * Case insensitive by default,
                  you may change this with the flag ``case_sensitive=True``.
                * Extension should be passed without leading dot
                  unless it's a part of the extension.
                * Pass :obj:`None` to filter files with no extension,
                  i.e. without a dot in the filename.

            Example:
                * ``Filters.document.file_extension("jpg")`` filters files with extension
                  ``".jpg"``.
                * ``Filters.document.file_extension(".jpg")`` filters files with extension
                  ``"..jpg"``.
                * ``Filters.document.file_extension("Dockerfile", case_sensitive=True)``
                  filters files with extension ``".Dockerfile"`` minding the case.
                * ``Filters.document.file_extension(None)`` filters files without a dot in the
                  filename.

[Poolitzer]

LGTM with the rendering fixes bibo suggested