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

Add pre-commit checks for python comprehensions #1764

Merged
merged 5 commits into from
Jul 1, 2021
Merged

Add pre-commit checks for python comprehensions #1764

merged 5 commits into from
Jul 1, 2021

Conversation

Anthchirp
Copy link
Member

This will now flag code such as

./tests/algorithms/spot_finding/test_per_image_analysis.py:64:16: C406 Unnecessary list literal - rewrite as a dict literal.
./tests/algorithms/spot_finding/test_per_image_analysis.py:89:16: C406 Unnecessary list literal - rewrite as a dict literal.
./tests/algorithms/image/filter/test_median.py:73:30: C414 Unnecessary list call within sorted().
./tests/algorithms/integration/profile/test_grid_sampler.py:47:13: C416 Unnecessary list comprehension - rewrite using list().

which have been fixed in this pull request.
Around 25 more remain.

@Anthchirp Anthchirp requested a review from ndevenish June 23, 2021 23:34
@codecov
Copy link

codecov bot commented Jun 24, 2021

Codecov Report

Merging #1764 (d270283) into main (d5f70f5) will increase coverage by 0.00%.
The diff coverage is 67.85%.

❗ Current head d270283 differs from pull request most recent head bb704dc. Consider uploading reports for the commit bb704dc to get more accurate results

@@           Coverage Diff           @@
##             main    #1764   +/-   ##
=======================================
  Coverage   67.22%   67.23%           
=======================================
  Files         616      616           
  Lines       69021    69012    -9     
  Branches     9618     9609    -9     
=======================================
  Hits        46399    46399           
+ Misses      20680    20671    -9     
  Partials     1942     1942           

- repo: https://github.com/PyCQA/isort.git
rev: 5.5.3
- repo: https://github.com/PyCQA/isort
rev: 5.6.4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a reason this was updated also, or just an opportunity to update? Why to 5.6.4?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an upportunity update to the most recent version that supports pre-commit <2.9, and that is also already referenced in other repositories such as ISPyB, zocalo, workflows, dlstbx, ...

@ndevenish
Copy link
Member

I've also gone through and updated the rest of the places where this warns. It looks like a good check. I also approve of consolidating configurations.

@ndevenish ndevenish merged commit dd661ec into main Jul 1, 2021
@ndevenish ndevenish deleted the pre-commit branch July 1, 2021 16:31
Anthchirp added a commit that referenced this pull request Jul 19, 2021
ndevenish pushed a commit that referenced this pull request Jul 21, 2021
- Single-source flake8 configuration
- Enable 'F541 f-string is missing placeholders' check
- Fix flake8 C4 errors left over from #1764
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