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

utilizing ruff for formatting & linting – #20 #31

Merged

Conversation

jGaboardi
Copy link
Collaborator

@jGaboardi jGaboardi commented May 31, 2024

utilizing ruff for formatting & linting – #20

  • run formatting & make initial PR
  • prelim review – actually is only a single change in base.py
  • run linting & re-push
  • ...

@jGaboardi jGaboardi self-assigned this May 31, 2024
@jGaboardi jGaboardi requested a review from carsonfarmer May 31, 2024 21:19
@jGaboardi
Copy link
Collaborator Author

Running ruff check fastpair with the following rules (standard in the PySAL ecosystem):

lint.select = ["E", "F", "W", "I", "UP", "N", "B", "A", "C4", "SIM", "ARG"]

results in:

fastpair/__init__.py:2:1: UP009 [*] UTF-8 encoding declaration is unnecessary
fastpair/__init__.py:16:19: F401 `.base.FastPair` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
fastpair/base.py:2:1: UP009 [*] UTF-8 encoding declaration is unnecessary
fastpair/base.py:36:1: UP010 [*] Unnecessary `__future__` imports `absolute_import`, `division`, `print_function` for target Python version
fastpair/base.py:36:1: I001 [*] Import block is un-sorted or un-formatted
fastpair/base.py:38:22: F401 [*] `operator.itemgetter` imported but unused
fastpair/base.py:46:7: N801 Class name `attrdict` should use CapWords convention
fastpair/base.py:50:14: UP008 Use `super()` instead of `super(__class__, self)`
fastpair/base.py:54:16: UP004 [*] Class `FastPair` inherits from `object`
fastpair/base.py:77:23: C408 Unnecessary `list` call (rewrite as a literal)
fastpair/base.py:101:21: F841 Local variable `res` is assigned to but never used
fastpair/base.py:119:16: E713 [*] Test for membership should be `not in`
fastpair/base.py:120:28: UP032 [*] Use f-string instead of `format` call
fastpair/base.py:124:16: E713 [*] Test for membership should be `not in`
fastpair/base.py:125:28: UP032 [*] Use f-string instead of `format` call
fastpair/test/test_fastpair.py:2:1: UP009 [*] UTF-8 encoding declaration is unnecessary
fastpair/test/test_fastpair.py:13:1: UP010 [*] Unnecessary `__future__` imports `absolute_import`, `division`, `print_function` for target Python version
fastpair/test/test_fastpair.py:13:1: I001 [*] Import block is un-sorted or un-formatted
fastpair/test/test_fastpair.py:17:44: F401 [*] `itertools.groupby` imported but unused
fastpair/test/test_fastpair.py:23:32: F401 [*] `scipy.unique` imported but unused
fastpair/test/test_fastpair.py:66:21: B905 [*] `zip()` without an explicit `strict=` parameter
fastpair/test/test_fastpair.py:82:5: N802 Function name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:99:26: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:106:24: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:117:24: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:130:24: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:137:29: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:144:42: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:153:38: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:171:44: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:179:9: E741 Ambiguous variable name: `l`
fastpair/test/test_fastpair.py:187:28: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:212:33: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:223:9: E741 Ambiguous variable name: `l`
fastpair/test/test_fastpair.py:224:9: F841 Local variable `res` is assigned to but never used
fastpair/test/test_fastpair.py:225:9: F841 Local variable `neigh` is assigned to but never used
fastpair/test/test_fastpair.py:270:25: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:280:13: F841 Local variable `myitem` is assigned to but never used
fastpair/test/test_fastpair.py:290:45: N803 Argument name `PointSet` should be lowercase
fastpair/test/test_fastpair.py:298:9: F841 Local variable `res` is assigned to but never used

There are actually far fewer linting issues than I anticipated and - from what I can tell - all are trivial fixes. Great job at writing future-proof code @carsonfarmer !

@codecov-commenter
Copy link

codecov-commenter commented Jun 2, 2024

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.2%. Comparing base (3d082b4) to head (7383b93).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main     #31     +/-   ##
========================================
- Coverage   100.0%   99.2%   -0.8%     
========================================
  Files           2       2             
  Lines         127     125      -2     
========================================
- Hits          127     124      -3     
- Misses          0       1      +1     
Files Coverage Δ
fastpair/__init__.py 100.0% <ø> (ø)
fastpair/base.py 99.2% <91.7%> (-0.8%) ⬇️

res = min(l, key=itemgetter(0))
neigh = fp.neighbors[new]
l_ = [(fp.dist(a, b), b) for a, b in zip(cycle([new]), ps)]
res = min(l_, key=itemgetter(0)) # noqa: F841
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't sure if res and neigh were actually needed for something, but not being testing so I left in and added logic for ruff to ignore.

Copy link
Owner

Choose a reason for hiding this comment

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

they were used in the following asserts, but have been commented out for reasons I am no longer aware of :P

Copy link
Owner

Choose a reason for hiding this comment

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

So we could either turn those back on, or just drop res and neigh I suspect.

fastpair/test/test_fastpair.py Show resolved Hide resolved
res = min(l, key=itemgetter(0))
neigh = fp.neighbors[new]
l_ = [(fp.dist(a, b), b) for a, b in zip(cycle([new]), ps)]
res = min(l_, key=itemgetter(0)) # noqa: F841
Copy link
Owner

Choose a reason for hiding this comment

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

they were used in the following asserts, but have been commented out for reasons I am no longer aware of :P

res = min(l, key=itemgetter(0))
neigh = fp.neighbors[new]
l_ = [(fp.dist(a, b), b) for a, b in zip(cycle([new]), ps)]
res = min(l_, key=itemgetter(0)) # noqa: F841
Copy link
Owner

Choose a reason for hiding this comment

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

So we could either turn those back on, or just drop res and neigh I suspect.

fastpair/test/test_fastpair.py Outdated Show resolved Hide resolved
fastpair/test/test_fastpair.py Outdated Show resolved Hide resolved
@jGaboardi jGaboardi merged commit 3cb9f3b into carsonfarmer:main Jun 5, 2024
1 check passed
@jGaboardi jGaboardi deleted the GH20_ruff_for_format_and_lint branch June 5, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants