Matching cython files: merge with python or support separately?

[webknjaz]

Most Python linters would work with Cython as well. I was using pydocstyle and thought that *.pyx files would be matched. Turned out they aren't.

I tracked it to calling identify and it returns just cython:

$ python -c 'import identify.identify; print(identify.identify.tags_from_path("src/pylibsshext/channel.pxd"))'
{'file', 'cython', 'text', 'non-executable'}
$ python -c 'import identify.identify; print(identify.identify.tags_from_path("src/pylibsshext/channel.pyx"))'
{'file', 'non-executable', 'text', 'cython'}

What can we do to improve this? Should Python mean Python+Cython+python-like? Should Cython be separate? If yes, should the docs promote the need to also add cython more actively?

[asottile]

I strongly disagree that most python linters would work -- cython has custom incompatible syntax

cython source is not python source so they cannot be merged (in the same way that pyi is kept separate, and that is even more like python!)

I don't think there's anything to document either, pre-commit may be written in python but it supports many programming languages and I don't think cython is special enough to warrant words in the docs.

[webknjaz]

I'm thinking along the lines of documenting the possible value for the files entry in hook definitions. Most folks probably don't even think about the possibility to add it in their configs.

[asottile]

files is already in the hook table

[webknjaz]

Oh, I meant types. Specifically, add an example mentioning cython type.

[asottile]

no I don't think so (as I said above), plus it wouldn't help since types: [python, cython] matches nothing

[asottile]

(and there's hundreds of other types, why should cython be special cased)

[webknjaz]

no I don't think so (as I said above), plus it wouldn't help since types: [python, cython] matches nothing

This is weird because I just added types: [python, cython] in a few places and it actually also matches pxd for me. But I'll check.

As for special-casing. It's just because if a linter supports Python but also Cython, if people mostly use Python only they tend to forget. I've integrated some things a few months ago and just now realized that some linters don't point to errors in Cython and only after some investigation it turned out that these files aren't matched. For example, flake8 can lint Cython with a few exceptions so I set up .flake8 with those per-file-ignores for Cython files but didn't even think that pre-commit also needs adjusting.

[webknjaz]

Here's where specifying two types seem to work: ansible/pylibssh@91bafcd

[asottile]

very easy to check if you're curious:

$ cat .pre-commit-config.yaml 
repos:
-   repo: meta
    hooks:
    -   id: identity
        name: identity (python, cython)
        types: [python, cython]
    -   id: identity
        name: identity (python)
        types: [python]
    -   id: identity
        name: identity (cython)
        types: [cython]
$ pre-commit  run --all-files
identity (python, cython)............................(no files to check)Skipped
- hook id: identity
identity (python)........................................................Passed
- hook id: identity
- duration: 0.03s

foo.py

identity (cython)........................................................Passed
- hook id: identity
- duration: 0.03s

foo.pyx

$ ls
foo.py  foo.pyx

[webknjaz]

Nvmd, I missed adding both python+cython in some places.

[asottile]

yeah definitely didn't work :P

isort....................................................(no files to check)Skipped
Add trailing commas......................................(no files to check)Skipped

[webknjaz]

(yeah, I'm watching you live)

[asottile]

(spooky)

[webknjaz]

Do you think I should switch to files: regex instead?

[asottile]

I would do multiple hooks instead of using files: that way you can still have the advantages of types

    hooks:
    -   id: pydocstyle
        name: pydocstyle (python)
    -   id: pydocstyle
        name: pydocstyle (cython)
        types: [cython]

[webknjaz]

Right, it crossed my mind too. But then if there's multiple settings it's too much copy-paste. Like with flake8 + a bunch of plugins.

[asottile]

up to you 🤷 -- if you override files you'll want to set types: [file] back to the default

[webknjaz]

Did you think of some types-like setting that wouldn't work XOR-way?

[asottile]

yes, during the original design there were use cases identified that required AND and I couldn't come up with any that required OR -- for that you'd check out #607

[webknjaz]

Well, now you have such a case...

[asottile]

yeah there's several others in #607 (which is why the issue exists!) -- feel free to chip in on the design there, that's mostly where it is currently stalled

I'm going to close this here since I believe this is "resolved" (as resolved as it could be) -- thanks for the issue!