Skip to content
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

Fix PytestDeprecationWarning #7

Closed
impredicative opened this issue Apr 8, 2020 · 10 comments · Fixed by #8
Closed

Fix PytestDeprecationWarning #7

impredicative opened this issue Apr 8, 2020 · 10 comments · Fixed by #8

Comments

@impredicative
Copy link

impredicative commented Apr 8, 2020

Thank you for this invaluable package. Lately I see this message:

venv/lib/python3.8/site-packages/pytest_mccabe.py:44: PytestDeprecationWarning: direct construction of McCabeItem has been deprecated, please use McCabeItem.from_parent
    return McCabeItem(path, parent, complexity)

Please see this similar issue with comments providing additional info.

@The-Compiler
Copy link
Owner

Thanks for the report! I'm not using this plugin myself anymore, so I likely won't have the time to look into this - a PR would be much appreciated though!

@MartinThoma
Copy link
Contributor

Do you use anything else which goes in a similar direction?

@The-Compiler
Copy link
Owner

@MartinThoma I run linting steps (such as mccabe) via flake8, and then run both pytest and flake8 (as well as others, such as pylint or mypy) via tox. IMHO, it's a bad idea to run linters via pytest as running a linter and running tests are two different kind of things - though I only realized that after writing this plugin.

@impredicative
Copy link
Author

impredicative commented Jul 6, 2020

It's a pretty good idea to run linters from pytest when there is a supported pytest plugin. If works well for so many linters, just not this one. This plugin is useless and a new one is needed.

@The-Compiler
Copy link
Owner

@impredicative Or, you know, you could contribute a fix (as I mentioned above) rather than complaining and I'd gladly review/merge PRs and cut a new release. Things like this is what drives open source maintainers to burnout. Nobody owes you anything in their free time - either take it, improve it, or go use something else.

@graingert
Copy link

@impredicative you can run also McCabe in pytest with https://pypi.org/project/pytest-flake8/

@MartinThoma
Copy link
Contributor

it's a bad idea to run linters via pytest as running a linter and running tests are two different kind of things

What is the disadvantage of running them both (test + linters) via pytest? I find it very handy to be able to do so. Usually, before I push stuff I run most things locally via pytest. I like to have one command to do it all.

@The-Compiler
Copy link
Owner

Just to clarify this again: I don't have a strict opinion on this - what I wrote above is based on my personal experience (mainly with my main project, qutebrowser) and happened back in 2015 (some of it before e.g. pytest-flake8 existed). If you're happy with running linters from pytest and it works out for you, I don't see an important reason to stop doing so.

Since I don't really remember the details on why I switched, I went digging into the qutebrowser repository a bit. I started with a custom script calling flake8 in January 2014, then switched to running linters via pytest in June 2015, and back to flake8 in January 2016.

From the top of my head, here are some thoughts about things which might or might not be an issue. Many of those wouldn't apply to pytest-flake8 I guess, just to the plugins calling linters directly:

  • They are rather uncommon (and not e.g. used by pytest itself), and thus bugs like Regression in 3.7.0: AttributeError for _collectfile pytest-dev/pytest#3742 can pop up. I think this was the main reason I switched to flake8 back then: There was some friction with those plugins (also with linter version upgrades), which was managed better by flake8.
  • There's a lot of logic (configuration, ignores, etc.) which flake8 takes care of, which is duplicated in those plugins. Contrary to flake8, there's no unification there, every linter is configured differently in some subtle ways.
  • Some editors support linters from the editor and annotating the results. If you use an exotic way of e.g. specifying ignores, they are unlikely to pick those up - since flake8 is much more common, they usually can run flake8 and deal with its config.
  • There are a lot of smaller linters which are implemented as flake8 plugins, and not available from pytest directly - see e.g. the ones I use, but there are thousands more
  • Most of their maintainers moved on to something like flake8 + tox (or perhaps flake8 + pytest), and don't really actively maintain those plugins anymore. That's not just me, see e.g. PytestDeprecationWarning printed with every test run asmeurer/pytest-flakes#26 or https://github.com/davidraleigh/pytest-pep8/graphs/contributors. Things seem to look better for things which aren't integrated into flake8, like pytest-pylint or pytest-mypy.
  • Those plugins are a bit in your way (or at least, used to be when I used them) when using advanced pytest features - I don't remember the exact details, but I remember having some commandline-flags (or perhaps things in my conftest.py?) which made a lot of sense for tests, but not so much for those plugins.
  • It's just a quite exotic way to do things, and might introduce additional friction for e.g. contributors to your project compared to e.g. tox. For a quick non-scientific comparison, see the PyPI stats for tox (2.3 million downloads in the past 30 days) vs. the stats for pytest-flake8 (175k downloads) vs. the stats for pytest-flakes (24k) or pytest-mccabe (12k).

Long story short: If you're using pytest-flake8 and that works for you, by all means, go ahead! If you're using pytest-{mccabe,pyflakes,pep8} individually, you might want to try pytest-flake8 instead.

I like to have one command to do it all.

FWIW that's the vision of tox: You clone a Python project (even one you've never used before), see a tox.ini, run tox, and tox will take care of setting up virtualenvs and running all the tools the project uses (and also runs tests on different Python versions).

Compare this to using e.g. pytest-flake8 (or pytest-mccabe for that matter): A new contributor to your project will need to set up a virtualenv, find out what test runner you're using (granted, pytest is pretty common nowadays), find out what plugins they need to install to run the tests (hopefully there's a requirements-dev.txt or so somewhere?), and then finally they can run them after creating a virtualenv and installing the dependencies manually.

Additionally, tox still allows you to clearly separate the two environments, if needed. If you just want to run flake8 without your tests, all it takes is a tox -e flake8. This also means you can clearly separate different jobs in a build matrix on CI, so that "the linter failed" is a separate build status from "the tests failed". You can get the same result with something like pytest --flakes --ignore=test --noconftest or so, but that's a lot more complicated than running either tox or tox -e <environment>.

(Again, that's all my personal take on it - if you have a different opinion, that's completely fine!)

@MartinThoma
Copy link
Contributor

Wow, thank you so much for taking the time! I didn't know that flake8 also has plugins. Some notes which others might like as well:

@The-Compiler
Copy link
Owner

Thanks to @MartinThoma in #8, this is now fixed and I just released pytest-mccabe 2.0 with the fix. It also drops support for older pytest/Python versions, see the README for a changelog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants