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 flake8 #450

Merged
merged 10 commits into from
Feb 1, 2022
Merged

Fix flake8 #450

merged 10 commits into from
Feb 1, 2022

Conversation

rodrigoalmeida94
Copy link
Collaborator

Description

Flake8 step is now lint (tox -e lint).

@srmsoumya
Copy link
Collaborator

@rodrigoalmeida94 There are a lot of flake8 errors, we have to filter which ones we would like to catch.

@rodrigoalmeida94
Copy link
Collaborator Author

rodrigoalmeida94 commented Jan 28, 2022

@srmsoumya In the ideal world, we would fix them all. I think this is unreasonable at the moment so I ignored the major ones that look relatively harmless:

    E203, # whitespace before ':' 
    E501, # line too long
    F401, # imported but unused
    C901, # is too complex
    E402, # module level import not at top of file (for docs)

These are the ones that are left (after applying black and isort), and I think it makes sense to address:

./tests/test_eval/evaluator_test.py:89:9: F841 local variable 'path_temp' is assigned to but never used
./tests/test_data/test_coco.py:26:9: E266 too many leading '#' for block comment
./tests/test_data/test_coco.py:65:9: E266 too many leading '#' for block comment
./solaris/tile/raster_tile.py:599:9: F821 undefined name 'cog_translate'
./solaris/preproc/pipesegment.py:415:19: F821 undefined name 'self'
./solaris/preproc/sar.py:565:9: E741 ambiguous variable name 'l'
./solaris/eval/vector.py:35:9: E741 ambiguous variable name 'l'
./solaris/eval/base.py:115:9: F841 local variable 'ground_truth_ids' is assigned to but never used

In any case we should fix these after merging and rebasing with #448 cc @rbavery

@rodrigoalmeida94
Copy link
Collaborator Author

I fixed the remaining linter errors so now CI should be green.

@rbavery I have added rio_cogeo dependency since in one of the functions we are using cog_translate- we can easily remove this, but then we should also remove the call to this function.

@rodrigoalmeida94 rodrigoalmeida94 marked this pull request as ready for review January 31, 2022 11:38
@rbavery
Copy link
Contributor

rbavery commented Jan 31, 2022

Let's keep it in as a dependency @rodrigoalmeida94 sicne it's currently used and we also want to better support cogs in the future

tox.ini Outdated Show resolved Hide resolved
@rodrigoalmeida94 rodrigoalmeida94 merged commit 92dd5e4 into feature/440-cicd-ghaction Feb 1, 2022
@rodrigoalmeida94 rodrigoalmeida94 deleted the fix-flake8 branch February 1, 2022 12:38
rodrigoalmeida94 pushed a commit that referenced this pull request Feb 8, 2022
* Add linting with flake8, black & isort

- Remove python-env from tox.ini files, use gh-actions matrix for
running different python versions
- Define tox runs as `tox -e [lint/format/flake8/test]`

* Change basepython to python3 from python3.9. Add test run with tox.

* Add python3.10 to the github runner matrix

* Use ubuntu-small osgeo container

- Tested github-actions locally using act.
- osgeo ubuntu-large images are 1.4 GB in size, takes some time to run this locally
- Replacing this with ubuntu-small runs it faster with no side effects

* Add release Actions workflow (#449)

* Uncomment tox

* Update job name to test

* Add release workflow only on main and with tag

* Fix flake8 (#450)

* lint is flake8

* Remove - in order to not ignore exit code

* Add extended ignores for flake8

* Rm unused vars

* Remove extra #

* Rm unused var

* Add rio-cogeo dependency

* Fix undefined variable, ignore ambigous name

* Rm flake8

* Fix unused imports F401

* Remove -

* Add cv2 dependencies

* Format

* Check if formatting passes

* Move black/isort/flake8 config to setup.cfg

* Add pre-commit config with isort, balck and flake8

* Add pre-commit dep

Co-authored-by: SRM <soumya@developmentseed.org>
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 this pull request may close these issues.

None yet

3 participants