Update guidelines for Python 3.11 and lint settings #7
Conversation
Reviewer's GuideThis PR updates the project requirements and linting guidelines to Python 3.11, introduces a script to verify that imports match Class Diagram for the new
|
| Change | Details | Files |
|---|---|---|
| Bump Python requirement to 3.11 in documentation |
|
README.md |
| Add repository guidelines document |
|
AGENTS.md |
| Introduce import-requirements verification script |
|
utils/verify_requirements.py |
| Relax Flake8 line-length and update lint instructions |
|
AGENTS.md |
| Reformat codebase with Black |
|
utils/ui_manager.pyutils/data_manager.pyutils/punch_counter.pyutils/calibration.pyutils/pose_detector.pymain.pytests/test_data_manager.pytests/test_punch_counter.py |
Tips and commands
Interacting with Sourcery
- Trigger a new review: Comment
@sourcery-ai reviewon the pull request. - Continue discussions: Reply directly to Sourcery's review comments.
- Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with@sourcery-ai issueto create an issue from it. - Generate a pull request title: Write
@sourcery-aianywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai titleon the pull request to (re-)generate the title at any time. - Generate a pull request summary: Write
@sourcery-ai summaryanywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment@sourcery-ai summaryon the pull request to
(re-)generate the summary at any time. - Generate reviewer's guide: Comment
@sourcery-ai guideon the pull
request to (re-)generate the reviewer's guide at any time. - Resolve all Sourcery comments: Comment
@sourcery-ai resolveon the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore. - Dismiss all Sourcery reviews: Comment
@sourcery-ai dismisson the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai reviewto trigger a new review!
Customizing Your Experience
Access your dashboard to:
- Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others. - Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.
Getting Help
- Contact our support team for questions or feedback.
- Visit our documentation for detailed guides and information.
- Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant verify_requirements.py
participant requirements.txt
participant Python Files
Developer->>verify_requirements.py: Run script
verify_requirements.py->>Python Files: Scan for imports
Python Files-->>verify_requirements.py: List of imported packages
verify_requirements.py->>requirements.txt: Read declared packages
requirements.txt-->>verify_requirements.py: List of required packages
verify_requirements.py->>Developer: Report missing packages or success
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
utils/pose_detector.py (2)
170-170: Rename unused loop variable to indicate intentional non-use.The loop variable
idxis not used within the loop body. Follow Python convention by prefixing with underscore.- for idx, x, y, confidence in keypoints: + for _idx, x, y, confidence in keypoints:🧰 Tools
🪛 Ruff (0.11.9)
170-170: Loop control variable
idxnot used within loop bodyRename unused
idxto_idx(B007)
113-185: Consider refactoring the draw_pose method to reduce complexity.The method has 16 local variables, which exceeds the recommended limit and may impact maintainability. Consider extracting helper methods for drawing connections and keypoints.
For example, you could extract:
_draw_connections(output_img, keypoint_dict, connections)_draw_keypoints(output_img, keypoints)This would improve readability and make the code more modular.
🧰 Tools
🪛 Ruff (0.11.9)
170-170: Loop control variable
idxnot used within loop bodyRename unused
idxto_idx(B007)
🪛 Pylint (3.3.7)
[refactor] 113-113: Too many local variables (16/15)
(R0914)
main.py (1)
159-179: Reduce unnecessaryelif. After abreak, followingelifblocks can be standaloneifstatements to improve clarity. Consider replacingelifwithif.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 160-179: Unnecessary "elif" after "break", remove the leading "el" from "elif"
(R1723)
utils/ui_manager.py (1)
35-43: Consider refactoring method signature.update_displaynow takes 7 parameters, exceeding typical limits. Group related args into a context object or dataclass to simplify the signature.🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 35-35: Too many arguments (7/5)
(R0913)
[refactor] 35-35: Too many positional arguments (7/5)
(R0917)
AGENTS.md (1)
17-17: Grammar nitpick. The header “## Environment Setup” repeats the verb “Follow” in consecutive instructions. Consider rephrasing for clarity.🧰 Tools
🪛 LanguageTool
[grammar] ~17-~17: You’ve repeated a verb. Did you mean to only write one of them?
Context: ... utils/verify_requirements.py ``` ## Formatting - Format code with Black before committing: ...(REPEATED_VERBS)
utils/punch_counter.py (1)
5-5: Remove unused numpy import.The
numpyimport is not used anywhere in this file and should be removed to clean up dependencies.-import numpy as np🧰 Tools
🪛 Ruff (0.11.9)
5-5:
numpyimported but unusedRemove unused import:
numpy(F401)
utils/verify_requirements.py (3)
16-17: Simplify nested if statement.The nested if statement can be simplified using a single condition with logical AND.
- elif isinstance(node, ast.ImportFrom): - if node.level == 0 and node.module: - packages.add(node.module.split(".")[0]) + elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module: + packages.add(node.module.split(".")[0])🧰 Tools
🪛 Ruff (0.11.9)
16-17: Use a single
ifstatement instead of nestedifstatements(SIM102)
36-36: Consider more robust exclusion logic.The current exclusion logic may miss virtual environments with different names. Consider using a more comprehensive approach.
- if py_file.name == "verify_requirements.py" or "venv" in py_file.parts: + if py_file.name == "verify_requirements.py" or any( + part in {"venv", ".venv", "env", ".env", "virtualenv"} + for part in py_file.parts + ):
29-29: Handle additional version specifier formats.The current parsing only handles
==and>=but could be enhanced to support other common formats like~=,>,<,!=.- pkgs.add(line.split("==")[0].split(">=")[0]) + # Split on various version specifiers + for sep in ["==", ">=", "<=", ">", "<", "~=", "!="]: + line = line.split(sep)[0] + pkgs.add(line.strip())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
AGENTS.md(1 hunks)README.md(2 hunks)main.py(4 hunks)tests/test_data_manager.py(3 hunks)tests/test_punch_counter.py(1 hunks)utils/calibration.py(4 hunks)utils/data_manager.py(3 hunks)utils/pose_detector.py(5 hunks)utils/punch_counter.py(3 hunks)utils/ui_manager.py(3 hunks)utils/verify_requirements.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
tests/test_data_manager.py (1)
utils/data_manager.py (4)
DataManager(12-230)create_new_session(57-61)save_session_data(63-110)get_historical_data(125-171)
tests/test_punch_counter.py (1)
utils/punch_counter.py (2)
_classify_punch_type(133-194)PunchCounter(12-245)
utils/calibration.py (1)
utils/pose_detector.py (1)
get_hand_keypoints(187-220)
utils/punch_counter.py (1)
utils/pose_detector.py (1)
get_hand_keypoints(187-220)
🪛 Ruff (0.11.9)
utils/pose_detector.py
5-5: os imported but unused
Remove unused import: os
(F401)
6-6: numpy imported but unused
Remove unused import: numpy
(F401)
170-170: Loop control variable idx not used within loop body
Rename unused idx to _idx
(B007)
utils/ui_manager.py
7-7: time imported but unused
Remove unused import: time
(F401)
8-8: datetime.timedelta imported but unused
Remove unused import: datetime.timedelta
(F401)
utils/calibration.py
6-6: numpy imported but unused
Remove unused import: numpy
(F401)
utils/data_manager.py
8-8: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
utils/punch_counter.py
5-5: numpy imported but unused
Remove unused import: numpy
(F401)
9-9: utils.pose_detector.PoseDetector imported but unused
Remove unused import: utils.pose_detector.PoseDetector
(F401)
utils/verify_requirements.py
16-17: Use a single if statement instead of nested if statements
(SIM102)
🪛 Pylint (3.3.7)
utils/pose_detector.py
[refactor] 113-113: Too many local variables (16/15)
(R0914)
utils/ui_manager.py
[refactor] 35-35: Too many arguments (7/5)
(R0913)
[refactor] 35-35: Too many positional arguments (7/5)
(R0917)
[refactor] 72-72: Too many arguments (6/5)
(R0913)
[refactor] 72-72: Too many positional arguments (6/5)
(R0917)
[refactor] 72-72: Too many local variables (24/15)
(R0914)
[refactor] 238-238: Too many local variables (16/15)
(R0914)
main.py
[refactor] 160-179: Unnecessary "elif" after "break", remove the leading "el" from "elif"
(R1723)
utils/punch_counter.py
[refactor] 12-12: Too many instance attributes (10/7)
(R0902)
[refactor] 133-133: Too many local variables (20/15)
(R0914)
🪛 LanguageTool
AGENTS.md
[grammar] ~17-~17: You’ve repeated a verb. Did you mean to only write one of them?
Context: ... utils/verify_requirements.py ``` ## Formatting - Format code with Black before committing: ...
(REPEATED_VERBS)
🔇 Additional comments (20)
README.md (1)
4-4: LGTM! Python version requirement consistently updated.The Python version badge and prerequisites section have been consistently updated to reflect the new minimum requirement of Python 3.11, aligning with the PR objectives.
Also applies to: 35-35
tests/test_punch_counter.py (1)
7-12: LGTM! Consistent string formatting applied.The conversion from single quotes to double quotes aligns with the repository-wide code style improvements. The test logic and assertions remain unchanged.
Also applies to: 19-25
tests/test_data_manager.py (1)
7-7: LGTM! Code formatting consistently improved.The stylistic changes including double quotes, trailing commas, and spacing improvements enhance code consistency without affecting test functionality.
Also applies to: 13-17, 22-22, 31-35, 44-44
utils/pose_detector.py (1)
4-11: LGTM! Formatting improvements enhance code readability.The consistent application of double quotes, improved spacing, and multi-line formatting aligns with the repository-wide style guidelines without affecting functionality.
Also applies to: 31-32, 44-45, 53-54, 61-62, 73-74, 84-86, 111-112, 125-126, 145-146, 168-169, 184-186, 219-221
🧰 Tools
🪛 Ruff (0.11.9)
5-5:
osimported but unusedRemove unused import:
os(F401)
6-6:
numpyimported but unusedRemove unused import:
numpy(F401)
utils/calibration.py (3)
15-21: Approve list formatting. Thecalibration_stepslist has been expanded into a multi-line, trailing-comma style for readability. No functional impact.
30-34: Approve dict formatting. Added trailing commas in thecalibration_datadict, aligning with Black formatting guidelines.
4-4: Skip whitespace-only changes. These blank-line adjustments don’t affect functionality.Also applies to: 44-44, 59-59
utils/data_manager.py (2)
36-50: Approve SQL formatting. Rewrote theCREATE TABLEandINSERTstatements as triple-quoted, multi-line strings with consistent indentation for improved readability.
155-170: Approve dictionary conversion. The rows fetched from the database are now mapped into dictionaries with consistent quoting and trailing commas. No logic changes.main.py (1)
1-158: Approve formatting improvements. Consistent use of double quotes, trailing commas, and multi-line argument lists aligns with the new style guidelines. No logic changes.🧰 Tools
🪛 Ruff (0.11.9)
5-5:
osimported but unusedRemove unused import:
os(F401)
6-6:
timeimported but unusedRemove unused import:
time(F401)
8-8:
numpyimported but unusedRemove unused import:
numpy(F401)
9-9:
tensorflowimported but unusedRemove unused import:
tensorflow(F401)
🪛 Pylint (3.3.7)
[refactor] 19-19: Too many instance attributes (14/7)
(R0902)
utils/ui_manager.py (1)
27-34: Approve dict formatting. Reformatted thepunch_colorsmapping with trailing commas and aligned entries per style guide.utils/punch_counter.py (6)
25-25: LGTM: Clean dictionary and deque formatting.The single-line formatting of dictionary and deque initializations improves readability while maintaining the same functionality.
Also applies to: 30-30, 36-36
52-52: LGTM: Improved sensitivity bounds enforcement.The addition of
max(5, ...)ensures the velocity threshold never goes below 5, preventing overly sensitive punch detection that could cause false positives.
91-91: LGTM: Improved mathematical expression formatting.The multi-line formatting of complex expressions enhances readability without changing the underlying math calculations.
Also applies to: 100-100
165-167: LGTM: Enhanced readability for conditional logic.The multi-line formatting of the boolean expression makes the left/right hand logic much clearer to understand.
170-182: LGTM: Well-formatted distance calculations.The multi-line formatting of mathematical expressions improves readability for the complex arm extension calculations.
241-243: LGTM: Improved print statement formatting.The multi-line formatting makes the debug output more readable while preserving the same functionality.
utils/verify_requirements.py (3)
8-19: LGTM: Robust import extraction logic.The AST-based approach correctly handles both
importandfrom ... importstatements, properly extracting top-level package names. The logic appropriately handles relative imports by checkingnode.level == 0.🧰 Tools
🪛 Ruff (0.11.9)
16-17: Use a single
ifstatement instead of nestedifstatements(SIM102)
22-30: LGTM: Solid requirements parsing with version handling.The parsing logic correctly handles comments, empty lines, and strips version specifiers using both
==and>=delimiters, which covers the most common requirement formats.
33-46: LGTM: Well-structured main function.The main execution flow is clean and handles the success/failure cases appropriately with proper exit codes. This aligns well with the PR objectives for improved dependency management.
| import os | ||
| import numpy as np |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused imports to clean up the code.
Static analysis correctly identifies that os and numpy are imported but never used in this file.
-import os
-import numpy as np
import tensorflow as tf📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import os | |
| import numpy as np | |
| import tensorflow as tf |
🧰 Tools
🪛 Ruff (0.11.9)
5-5: os imported but unused
Remove unused import: os
(F401)
6-6: numpy imported but unused
Remove unused import: numpy
(F401)
🤖 Prompt for AI Agents
In utils/pose_detector.py around lines 5 to 6, the imports for 'os' and 'numpy'
are unused and should be removed to clean up the code. Delete the lines
importing these modules to eliminate unnecessary dependencies and improve code
clarity.
| """ | ||
|
|
||
| import cv2 | ||
| import numpy as np |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused numpy import. The numpy module is imported but never referenced in this file. Dropping it will clean up dependencies.
🧰 Tools
🪛 Ruff (0.11.9)
6-6: numpy imported but unused
Remove unused import: numpy
(F401)
🤖 Prompt for AI Agents
In utils/calibration.py at line 6, the numpy import is unused and should be
removed to clean up dependencies and avoid unnecessary imports.
| import os | ||
| import json | ||
| import time | ||
| from datetime import datetime |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused datetime import. The datetime module is not used in this file.
🧰 Tools
🪛 Ruff (0.11.9)
8-8: datetime.datetime imported but unused
Remove unused import: datetime.datetime
(F401)
🤖 Prompt for AI Agents
In utils/data_manager.py at line 8, the datetime module is imported but not used
anywhere in the file. Remove the line importing datetime to clean up unused
imports and improve code clarity.
| import cv2 | ||
| import numpy as np | ||
| import time | ||
| from datetime import datetime, timedelta |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused timedelta import. datetime.timedelta is never referenced.
🧰 Tools
🪛 Ruff (0.11.9)
8-8: datetime.timedelta imported but unused
Remove unused import: datetime.timedelta
(F401)
🤖 Prompt for AI Agents
In utils/ui_manager.py at line 8, the timedelta import from the datetime module
is unused. Remove timedelta from the import statement so it only imports
datetime.
|
|
||
| import cv2 | ||
| import numpy as np | ||
| import time |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused time import. The time module is not utilized in this file.
🧰 Tools
🪛 Ruff (0.11.9)
7-7: time imported but unused
Remove unused import: time
(F401)
🤖 Prompt for AI Agents
In utils/ui_manager.py at line 7, the time module is imported but not used
anywhere in the file. Remove the import statement for time to clean up the code
and avoid unnecessary imports.
| - Use the default line length (88 characters) and 4‑space indentation. | ||
|
|
There was a problem hiding this comment.
Inconsistent line-length settings. The Formatting section mandates Black’s default (88 chars) while Linting uses --max-line-length=120. Align these values to avoid confusion.
Also applies to: 27-28
🤖 Prompt for AI Agents
In AGENTS.md around lines 22 to 23 and 27 to 28, the line length settings are
inconsistent between the Formatting section (88 characters) and the Linting
section (120 characters). Update the Linting section to use the default Black
line length of 88 characters to align both settings and avoid confusion.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @tblakex01 - I've reviewed your changes - here's some feedback:
- CI workflows/tests should be updated to run against Python 3.11 to reflect the new requirement.
- AGENTS.md still mentions Black’s default line-length of 88, but flake8 is set to 120 – please align the guidelines.
- Consider integrating verify_requirements.py into your CI pipeline or Makefile so missing imports are caught automatically.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| imports.update(gather_imports(py_file)) | ||
|
|
||
| requirements = load_requirements(REPO_ROOT / "requirements.txt") | ||
| missing = sorted(pkg for pkg in imports if pkg not in requirements) |
There was a problem hiding this comment.
issue (bug_risk): Imported standard library modules may be incorrectly flagged as missing.
The script should filter out standard library modules, for example by using 'sys' or 'stdlib_list', to avoid false positives when checking for missing packages.
|
|
||
|
|
||
| def gather_imports(file_path: Path): | ||
| with file_path.open("r") as f: |
There was a problem hiding this comment.
suggestion: No error handling for file read or parse errors.
Add error handling to skip unreadable or unparsable files and log the issue, so the script continues processing other files.
| output = capsys.readouterr().out | ||
| assert "Failed to write backup file" in output | ||
| assert len(list(data_dir.glob('session_*.json'))) == 0 | ||
| assert len(list(data_dir.glob("session_*.json"))) == 0 |
There was a problem hiding this comment.
suggestion (code-quality): Simplify sequence length comparison (simplify-len-comparison)
| assert len(list(data_dir.glob("session_*.json"))) == 0 | |
| assert not list(data_dir.glob("session_*.json")) |
|
|
||
| # Add calibration step text | ||
| if self.calibration_stage < len(self.calibration_steps): | ||
| current_step = self.calibration_steps[self.calibration_stage] |
There was a problem hiding this comment.
issue (code-quality): Extract code out into method (extract-method)
| # Convert to list of dictionaries | ||
| historical_data = [] | ||
| for session in sessions: | ||
| historical_data.append({ | ||
| 'date': session[0], | ||
| 'duration': session[1], | ||
| 'total_punches': session[2], | ||
| 'punches_per_minute': session[3], | ||
| 'punch_types': { | ||
| 'jab': session[4], | ||
| 'cross': session[5], | ||
| 'hook': session[6], | ||
| 'uppercut': session[7] | ||
| historical_data.append( | ||
| { | ||
| "date": session[0], | ||
| "duration": session[1], | ||
| "total_punches": session[2], | ||
| "punches_per_minute": session[3], | ||
| "punch_types": { | ||
| "jab": session[4], | ||
| "cross": session[5], | ||
| "hook": session[6], | ||
| "uppercut": session[7], | ||
| }, | ||
| } | ||
| }) | ||
| ) | ||
|
|
||
| return historical_data |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Convert for loop into list comprehension (
list-comprehension) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| # Convert to list of dictionaries | |
| historical_data = [] | |
| for session in sessions: | |
| historical_data.append({ | |
| 'date': session[0], | |
| 'duration': session[1], | |
| 'total_punches': session[2], | |
| 'punches_per_minute': session[3], | |
| 'punch_types': { | |
| 'jab': session[4], | |
| 'cross': session[5], | |
| 'hook': session[6], | |
| 'uppercut': session[7] | |
| historical_data.append( | |
| { | |
| "date": session[0], | |
| "duration": session[1], | |
| "total_punches": session[2], | |
| "punches_per_minute": session[3], | |
| "punch_types": { | |
| "jab": session[4], | |
| "cross": session[5], | |
| "hook": session[6], | |
| "uppercut": session[7], | |
| }, | |
| } | |
| }) | |
| ) | |
| return historical_data | |
| return [ | |
| { | |
| "date": session[0], | |
| "duration": session[1], | |
| "total_punches": session[2], | |
| "punches_per_minute": session[3], | |
| "punch_types": { | |
| "jab": session[4], | |
| "cross": session[5], | |
| "hook": session[6], | |
| "uppercut": session[7], | |
| }, | |
| } | |
| for session in sessions | |
| ] |
| total_punches = result[1] or 1 # Avoid division by zero | ||
|
|
||
| summary = { |
There was a problem hiding this comment.
issue (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
| verticalalignment="center", | ||
| transform=ax2.transAxes, | ||
| ) | ||
| ax2.set_title("Historical Data") |
There was a problem hiding this comment.
issue (code-quality): Hoist repeated code outside conditional statement [×2] (hoist-statement-from-if)
| missing = sorted(pkg for pkg in imports if pkg not in requirements) | ||
| if missing: |
There was a problem hiding this comment.
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression)
| missing = sorted(pkg for pkg in imports if pkg not in requirements) | |
| if missing: | |
| if missing := sorted(pkg for pkg in imports if pkg not in requirements): |
Summary
Testing
black --check .flake8(fails: command not found)pytest -q(fails: ModuleNotFoundError: No module named 'numpy')https://chatgpt.com/codex/tasks/task_e_6845b85aeaa0833295eae381bbfe322a
Summary by Sourcery
Require Python 3.11, introduce a dependency verification tool, relax linting rules, and standardize code style with Black. Update documentation and badges to reflect these new guidelines.
New Features:
utils/verify_requirements.py) to validate that all imports are listed inrequirements.txtEnhancements:
Documentation:
README.mdAGENTS.mdwith repository guidelines for environment setup, formatting, linting, testing, and pull request standardsSummary by CodeRabbit
Documentation
Chores
Style
Refactor
Bug Fixes