Skip to content
This repository has been archived by the owner on Jan 29, 2024. It is now read-only.

Test linting of "data_and_models/" #375

Closed
FrancescoCasalegno opened this issue May 25, 2021 · 2 comments · Fixed by #393
Closed

Test linting of "data_and_models/" #375

FrancescoCasalegno opened this issue May 25, 2021 · 2 comments · Fixed by #393

Comments

@FrancescoCasalegno
Copy link
Contributor

Extend lint tests to the content of data_and_models/.

Any file that are taken from other repos as-is (e.g. this script) should not be covered by this test.

@Stannislav
Copy link
Contributor

Stannislav commented May 26, 2021

I investigated into this and here's what I can report:

The good - flake8 and isort

The ignore paths for these can be configured directly in tox.ini. One can provide lists of files and paths to ignore

;tox.ini
[flake8]
extend-exclude =
    data_and_models/pipelines/ner
    data_and_models/pipelines/relation_extraction

[isort]
extend_skip =
    data_and_models/pipelines/ner
    data_and_models/pipelines/relation_extraction

The bad - black and mypy

Both require one single string with a regular expression that determines all paths that should be ignored.

On top of that, black wants its configuration in pyproject.toml.

# pyproject.toml
[tool.black]
extend-exclude = "data_and_models/pipelines/(sentence_embedding|ner)"

mypy can actually be configured to read its config from tox.ini via mypy --config-file tox.ini <files> (why are we not doign that?)

;tox.ini
[mypy]
exclude = benchmarks/conftest.py|data_and_models/pipelines/(sentence_embedding|ner)

The ugly - bandit

No matter how many times I try to configure bandit, I always fail. Already from bandit --help

  -c CONFIG_FILE, --configfile CONFIG_FILE
                        optional config file to use for selecting plugins and
                        overriding defaults

  --ini INI_PATH        path to a .bandit file that supplies command line
                        arguments

So why is it necessary to use two different config files? The online docs provide a little bit more insight into the format of the config file specified via the -c option: it supposed to be a YAML file (!) And no matter how hard I looked I wasn't able to find a parameter in this file that would exclude files.

There's also a discussion about configuring bandit here: PyCQA/bandit#396 enjoy...

In the discussion above someone mentiones that they've managed to configure bandit via tox.ini I tried and it didn't work for me. Let me know if you find out how to do that.

Actually I just checked the output of bandit --help again and I see

  -x EXCLUDED_PATHS, --exclude EXCLUDED_PATHS
                        comma-separated list of paths (glob patterns
                        supported) to exclude from scan (note that these are
                        in addition to the excluded paths provided in the
                        config file) (default:
                        .svn,CVS,.bzr,.hg,.git,__pycache__,.tox,.eggs,*.egg)

It says "(note that these are in addition to the excluded paths provided in the config file)" but I really couldn't find such an option in the YAML file provided via the -c flag :(

So for now I only can exclude paths only via a the command line --exclude flag.

@FrancescoCasalegno
Copy link
Contributor Author

As discussed with the rest of the team, let's try to create a vendor/ directory for all the code that we vendor and therefore we don't want to test.

EmilieDel added a commit that referenced this issue Jul 16, 2021
Co-authored-by: Pierre-Alexandre Fonta <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants