Remove `pytest_flake8` plugin and use `flake8` instead #1722

valeriupredoi · 2022-09-15T13:19:13Z

Description

Discussed this and @bouweandela provided a working solution to the problem of the pytest_flake8 plugin being more and more at odds with the mother package flake8. I think we should change the documentation too, tell users to run flake8 before a push, but maybe not in this PR, what do you think @bouweandela ? We should also visit the containers 📦

Closes #1699

Link to documentation:

Before you get started

☝ Create an issue to discuss what you are going to do

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

🛠 This pull request has a descriptive title and labels
🛠 Any changed dependencies have been added or removed correctly
🛠 All checks below this pull request were successful

To help with the number pull requests:

🙏 We kindly ask you to review two other open pull requests in this repository

codecov · 2022-09-15T13:40:26Z

Codecov Report

Merging #1722 (286a0b7) into main (55d630c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1722   +/-   ##
=======================================
  Coverage   91.47%   91.47%           
=======================================
  Files         206      206           
  Lines       11204    11204           
=======================================
  Hits        10249    10249           
  Misses        955      955

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

valeriupredoi · 2022-09-15T14:21:20Z

so the good news is that flake8 runs fine on me local machine (as well as the CI machines) but produces no progress bar/dots/flying cats etc - when it finishes it just exists the prompt - the user won't know what the heck's going on, @bouweandela know a way to show some progress on screen? Like mamba - shooting lasers, now that's cool 🌟

valeriupredoi · 2022-09-15T14:26:07Z

Ha! Now that's an odd one - get rid of a pytest plugin, immediately run into a HDF5 segfault, well that escalated quickly 🤣

schlunma · 2022-09-15T14:31:35Z

Ha! Now that's an odd one - get rid of a pytest plugin, immediately run into a HDF5 segfault, well that escalated quickly rofl

😃

@taly-pereira ran into the same problem today with a recipe that ran fine for me. We think that a new hdf5 version might be the problem, as I am using

hdf5                     1.12.1         mpi_mpich_h08b82f9_4   conda-forge

whereas she uses (freshly installed environment from today)

hdf5                      1.12.2          mpi_mpich_h08b82f9_0    conda-forge

We are currently downgrading her version to check if that fixes the problem...

EDIT: downgrading to 1.12.1 fixed the problem!

valeriupredoi · 2022-09-15T14:38:20Z

cheers aplenty @schlunma - I was beginning to think the "butterfly effect" concept 😆 That test is currently stalled (taking forever and half) on my machine, so it's definitely something bellyup in HDF5 - if HDF5 is hosed then queue the angry issues on their github repo since everyone and their dog uses it 🍿

EDIT - that failing test passed on my machine with hdf5 1.12.2 mpi_mpich_h5d83325_0 conda-forge - but that's a different hash

EDI2: even that hash (and the same hash from the CI machine) ran a couple times more with -n 2 causes segfaults - HDF5 1.12.2 is el hosed

valeriupredoi · 2022-09-15T14:44:42Z

@schlunma read the EDIT2 above - HDF5 ist kaput

schlunma · 2022-09-15T14:50:46Z

Weird. I haven't found anything on their GitHub...it also looks like 1.12.2 is not the latest version

valeriupredoi · 2022-09-15T14:52:27Z

it is https://anaconda.org/conda-forge/hdf5/files - but it's fairly old and we've not seen anything weird until today?

valeriupredoi · 2022-09-15T14:55:49Z

it's netCDF4 that's the problem - updated 5h ago https://anaconda.org/conda-forge/netcdf4

schlunma · 2022-09-15T15:15:31Z

Ahh, yes, that makes sense!

schlunma · 2022-09-15T15:24:22Z

Maybe pin it to !=1.6.1 so that a fixed version gets pulled in automatically in the future?

valeriupredoi · 2022-09-15T15:34:55Z

yis, doing that now 🍺

.github/workflows/create-condalock-file.yml

bouweandela · 2022-09-15T19:30:37Z

I think we should change the documentation too, tell users to run flake8 before a push, but maybe not in this PR, what do you think @bouweandela ?

We have pretty extensive advice here aleady: https://docs.esmvaltool.org/projects/esmvalcore/en/latest/contributing.html#code-quality, including the advice to use pre-commit, which runs flake8 as well as a bunch of other useful checks.

valeriupredoi · 2022-09-16T12:26:32Z

I think we should change the documentation too, tell users to run flake8 before a push, but maybe not in this PR, what do you think @bouweandela ?

We have pretty extensive advice here aleady: https://docs.esmvaltool.org/projects/esmvalcore/en/latest/contributing.html#code-quality, including the advice to use pre-commit, which runs flake8 as well as a bunch of other useful checks.

oh yeah I knew about that - very nice indeed - but was wondering if it'd be worth it to tell an (advanced) developer how to check the code standards quality across the package with flake8 -j X?

bouweandela · 2022-09-16T12:58:03Z

No: we're already recommending prospector, which will probably tell you about all flake8 issues and more, and then we're recommending pre-commit, so I think that's enough. People already complain that our docs are too lengthy.

valeriupredoi · 2022-09-16T13:06:04Z

OK fair enough. That who complains the docs are too long has not read the fsspec documentation 😁

.circleci/config.yml

setup.py

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>

…/ESMValCore into remove_pytest_flake8

bouweandela · 2022-09-16T16:15:38Z

Looks good to me now! Only when I run flake8 on my computer I get a lot of errors coming from files in the .eggs directory. Do you think it would make sense to add that to the list of directories to ignore?

valeriupredoi · 2022-09-20T12:00:46Z

cheers Bouwe! I don't have any eggs, nor does the CI - where do you get your eggs from, bro? 🥚

bouweandela · 2022-09-20T12:21:59Z

It is created by setuptools, e.g. if you ever ran python setup.py develop or similar, you'll have it. Let's leave if for now.

valeriupredoi · 2022-09-20T12:29:47Z

It is created by setuptools, e.g. if you ever ran python setup.py develop or similar, you'll have it. Let's leave if for now.

but of course, silly me - it's just that I've not run python setup.py test/install etc in eons and I thought nobody else still does it; OK let's postpone, now who's using antiquated stuff 😁 Cheers for the review, bud! 🍺

@bouweandela

## Description  Sister PR to ESMValGroup/ESMValCore#1722 - quite needed since the conda-forge package needed some involved pinning of the flake8 and pytest-flake8 packages. Discussed this and @bouweandela provided a working solution to the problem of the `pytest_flake8` plugin being more and more at odds with the mother package `flake8`. Currently the main `flake8` package (and only one as of this PR) is pinned to <5, there are still some issues with upper versions but we should have in mind to unpin and test in the future. - Closes #2897 - Link to documentation: No need for it, no change to actual docs * * * ## Before you get started  - [x] [☝ Create an issue](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#contributing-code-and-documentation) to discuss what you are going to do ## Checklist It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the [🛠 Technical][1] or [🧪 Scientific][2] review.  [1]: https://docs.esmvaltool.org/en/latest/community/review.html#technical-review [2]: https://docs.esmvaltool.org/en/latest/community/review.html#scientific-review - [x] [🛠][1] This pull request has a [descriptive title](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#pull-request-title) - [x] [🛠][1] The [list of authors](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#list-of-authors) is up to date - [x] [🛠][1] Any changed dependencies have been [added or removed correctly](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#dependencies) - [x] [🛠][1] All [checks below this pull request](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#pull-request-checks) were successful Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>

valeriupredoi added 3 commits September 15, 2022 14:11

remove pytest_flake8 dep

f69d6db

add call to flake8 command and test here

140a688

add call to flake8

e78a22e

valeriupredoi added enhancement New feature or request installation Installation problem testing labels Sep 15, 2022

valeriupredoi requested a review from bouweandela September 15, 2022 13:19

valeriupredoi added 2 commits September 15, 2022 14:25

tell flake8 to stfu

1bad213

use exclude instead

32d2602

ups forgot flake8 pytest opt on

ae61a49

valeriupredoi added 2 commits September 15, 2022 15:59

pin netcdf4

b73a47e

pin netcdf4

3f6c43e

valeriupredoi added 2 commits September 15, 2022 16:36

pin netcdf4 restrictive

fa14195

pin netcdf4 restrictive

a328ab1

bouweandela reviewed Sep 15, 2022

View reviewed changes

.github/workflows/create-condalock-file.yml Outdated Show resolved Hide resolved

valeriupredoi added 2 commits September 16, 2022 14:32

same for GA tests

3c0a9aa

upshift for pytest since Circle is not retarded like GH machines

86b6249

bouweandela reviewed Sep 16, 2022

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

bouweandela reviewed Sep 16, 2022

View reviewed changes

setup.py Outdated Show resolved Hide resolved

valeriupredoi and others added 4 commits September 16, 2022 16:10

Update .circleci/config.yml

b421f1e

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>

turn off ga tests here

001f4fd

Merge branch 'main' into remove_pytest_flake8

d5e5052

Merge branch 'remove_pytest_flake8' of https://github.com/ESMValGroup…

286a0b7

…/ESMValCore into remove_pytest_flake8

bouweandela approved these changes Sep 20, 2022

View reviewed changes

valeriupredoi merged commit 640280b into main Sep 20, 2022

valeriupredoi deleted the remove_pytest_flake8 branch September 20, 2022 12:30

valeriupredoi mentioned this pull request Nov 1, 2022

Remove pytest-flake8 and call the use of flake8 straight ESMValGroup/ESMValTool#2904

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `pytest_flake8` plugin and use `flake8` instead #1722

Remove `pytest_flake8` plugin and use `flake8` instead #1722

valeriupredoi commented Sep 15, 2022 •

edited

Loading

codecov bot commented Sep 15, 2022 •

edited

Loading

valeriupredoi commented Sep 15, 2022

valeriupredoi commented Sep 15, 2022

schlunma commented Sep 15, 2022 •

edited

Loading

valeriupredoi commented Sep 15, 2022 •

edited

Loading

valeriupredoi commented Sep 15, 2022

schlunma commented Sep 15, 2022

valeriupredoi commented Sep 15, 2022

valeriupredoi commented Sep 15, 2022

schlunma commented Sep 15, 2022

schlunma commented Sep 15, 2022 •

edited

Loading

valeriupredoi commented Sep 15, 2022

bouweandela commented Sep 15, 2022

valeriupredoi commented Sep 16, 2022

bouweandela commented Sep 16, 2022

valeriupredoi commented Sep 16, 2022

bouweandela commented Sep 16, 2022

valeriupredoi commented Sep 20, 2022

bouweandela commented Sep 20, 2022

valeriupredoi commented Sep 20, 2022

Remove pytest_flake8 plugin and use flake8 instead #1722

Remove pytest_flake8 plugin and use flake8 instead #1722

Conversation

valeriupredoi commented Sep 15, 2022 • edited Loading

Description

Before you get started

Checklist

codecov bot commented Sep 15, 2022 • edited Loading

Codecov Report

valeriupredoi commented Sep 15, 2022

valeriupredoi commented Sep 15, 2022

schlunma commented Sep 15, 2022 • edited Loading

valeriupredoi commented Sep 15, 2022 • edited Loading

valeriupredoi commented Sep 15, 2022

schlunma commented Sep 15, 2022

valeriupredoi commented Sep 15, 2022

valeriupredoi commented Sep 15, 2022

schlunma commented Sep 15, 2022

schlunma commented Sep 15, 2022 • edited Loading

valeriupredoi commented Sep 15, 2022

bouweandela commented Sep 15, 2022

valeriupredoi commented Sep 16, 2022

bouweandela commented Sep 16, 2022

valeriupredoi commented Sep 16, 2022

bouweandela commented Sep 16, 2022

valeriupredoi commented Sep 20, 2022

bouweandela commented Sep 20, 2022

valeriupredoi commented Sep 20, 2022

Remove `pytest_flake8` plugin and use `flake8` instead #1722

Remove `pytest_flake8` plugin and use `flake8` instead #1722

valeriupredoi commented Sep 15, 2022 •

edited

Loading

codecov bot commented Sep 15, 2022 •

edited

Loading

schlunma commented Sep 15, 2022 •

edited

Loading

valeriupredoi commented Sep 15, 2022 •

edited

Loading

schlunma commented Sep 15, 2022 •

edited

Loading