precommit: Add config, apply changes #72
Conversation
|
Warning Rate limit exceeded@echarrod has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe pull request introduces a comprehensive pre-commit hooks configuration alongside code modernisation across the codebase. A new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span numerous files with predominantly homogeneous modifications (systematic quote standardisation and formatting consistency), which reduces cognitive overhead. However, the scope includes new configuration files, updated public method signatures, tool configurations across multiple files, and refactored constructor logic in Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.pre-commit-config.yaml (1)
68-75: Consider making pytest optional rather than always running.The
always_run: trueflag causes pytest to run on every commit, even when no Python files have changed. This can slow down the commit process significantly, especially as the test suite grows.Consider this alternative configuration:
- repo: local hooks: - id: pytest name: pytest entry: pytest language: system - pass_filenames: false - always_run: true + types: [python] + pass_filenames: falseThis will only run pytest when Python files are modified, whilst still maintaining test coverage for code changes.
setup.py (1)
19-33: Consider reducing duplication in extras_require.The test dependencies (
pytest,pytest-cov,requests_mock) are duplicated in both the "test" and "dev" extras. Whilst this works, it creates maintenance overhead.Consider this approach to reduce duplication:
_test_deps = ["pytest", "pytest-cov", "requests_mock"] extras_require={ "test": _test_deps, "dev": _test_deps + [ "pre-commit", "black", "isort", "flake8", "flake8-docstrings", "flake8-bugbear", "bandit[toml]", ], },This ensures both extras stay synchronised and reduces the risk of inconsistencies.
luno_python/base_client.py (1)
63-89: Consider modernising exception message formatting.Whilst the quote standardisation is consistent, line 89 still uses
%formatting instead of an f-string like line 105. For consistency with the rest of the modernisation effort, consider updating it.Optionally, the exception handling could be improved per static analysis hints:
- except JSONDecodeError: - raise Exception("luno: unknown API error (%s)" % res.status_code) + except JSONDecodeError as exc: + raise Exception(f"luno: unknown API error ({res.status_code})") from excThis adds exception chaining for better debugging and uses consistent f-string formatting. However, replacing
Exceptionwith a custom exception class would be a breaking change for existing error handling code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.pre-commit-config.yaml(1 hunks)CONTRIBUTING.md(1 hunks)LICENSE.txt(0 hunks)README.md(0 hunks)examples/readonly.py(2 hunks)luno_python/__init__.py(1 hunks)luno_python/base_client.py(4 hunks)luno_python/client.py(38 hunks)pyproject.toml(1 hunks)setup.cfg(1 hunks)setup.py(1 hunks)tests/test_client.py(2 hunks)
💤 Files with no reviewable changes (2)
- LICENSE.txt
- README.md
🧰 Additional context used
📓 Path-based instructions (3)
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use requests_mock for HTTP mocking in tests: mount the adapter on the session and register URIs with adapter.register_uri(...).
Files:
tests/test_client.py
luno_python/base_client.py
📄 CodeRabbit inference engine (AGENTS.md)
luno_python/base_client.py: Authenticate requests using HTTP Basic Auth with api_key_id and api_key_secret in the BaseClient.
In BaseClient.do(), parse JSON responses and raise APIError when the API returns an error_code/error.
Files:
luno_python/base_client.py
luno_python/client.py
📄 CodeRabbit inference engine (AGENTS.md)
luno_python/client.py: For API methods in luno_python/client.py, follow the existing docstring format including HTTP method, path, permissions, and parameter descriptions.
Each API method in luno_python/client.py must construct a req dict with parameters and call self.do() to perform the HTTP request.
Use ":type param: type_name" style type hints in docstrings for API methods to maintain Python 2 compatibility.
Use {param_name} syntax for path parameters in endpoint paths within API method definitions.
For GET requests, pass query parameters via the params field of the request.
Files:
luno_python/client.py
🧠 Learnings (2)
📚 Learning: 2025-10-23T11:07:39.142Z
Learnt from: CR
PR: luno/luno-python#0
File: AGENTS.md:0-0
Timestamp: 2025-10-23T11:07:39.142Z
Learning: Applies to luno_python/client.py : For API methods in luno_python/client.py, follow the existing docstring format including HTTP method, path, permissions, and parameter descriptions.
Applied to files:
luno_python/base_client.pyluno_python/client.py
📚 Learning: 2025-10-23T11:07:39.142Z
Learnt from: CR
PR: luno/luno-python#0
File: AGENTS.md:0-0
Timestamp: 2025-10-23T11:07:39.142Z
Learning: Applies to luno_python/client.py : Each API method in luno_python/client.py must construct a req dict with parameters and call self.do() to perform the HTTP request.
Applied to files:
luno_python/base_client.py
🧬 Code graph analysis (4)
tests/test_client.py (3)
luno_python/base_client.py (4)
set_auth(36-43)set_base_url(45-52)set_timeout(54-61)do(63-89)luno_python/client.py (1)
get_balances(159-190)luno_python/error.py (1)
APIError(1-4)
examples/readonly.py (1)
luno_python/client.py (10)
get_tickers(378-393)get_ticker(363-376)get_order_book(292-307)list_trades(514-535)get_candles(192-216)get_balances(159-190)list_user_trades(600-651)get_fee_info(218-231)get_funding_address(233-253)list_withdrawals(653-670)
luno_python/base_client.py (1)
luno_python/error.py (1)
APIError(1-4)
luno_python/client.py (1)
luno_python/base_client.py (1)
do(63-89)
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~24-~24: Possible missing comma found.
Context: ...e in editable mode with all development dependencies including testing tools and pre-commit ...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Ruff (0.14.1)
tests/test_client.py
21-21: Possible hardcoded password assigned to: "api_key_secret"
(S105)
34-34: Do not assert blind exception: Exception
(B017)
luno_python/base_client.py
87-87: Consider moving this statement to an else block
(TRY300)
89-89: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
89-89: Create your own exception
(TRY002)
🔇 Additional comments (26)
setup.cfg (1)
5-5: LGTM!The trailing newline addition aligns with the end-of-file-fixer hook introduced in the pre-commit configuration.
pyproject.toml (2)
21-23: LGTM!The Bandit configuration appropriately excludes test directories and skips B101 (assert_used), which is sensible for a project with tests.
25-30: LGTM!The Pytest configuration correctly sets up test discovery patterns and includes coverage reporting for the
luno_pythonpackage.CONTRIBUTING.md (4)
9-15: LGTM!The virtual environment setup instructions are clear and follow Python best practices.
17-25: LGTM!The updated dependency installation instructions correctly reference the new
devextras, which include all necessary development tools.
26-60: LGTM!The pre-commit hooks documentation is comprehensive, explaining installation, manual usage, what each hook does, and how to skip hooks when necessary. This provides excellent guidance for contributors.
62-66: LGTM!The test running instructions are concise and appropriate.
examples/readonly.py (1)
6-75: LGTM!The quote standardisation from single to double quotes throughout the file aligns with the project's formatting standards enforced by Black. The logic and functionality remain unchanged.
luno_python/client.py (7)
21-35: LGTM!Quote standardisation is applied consistently. The method structure follows coding guidelines: constructs a req dict and calls self.do() to perform the HTTP request.
159-190: LGTM!Quote standardisation is consistent throughout the method, including dictionary keys and string literals. The account filtering logic is preserved correctly.
600-651: LGTM!The multi-line signature reformatting improves readability for this method with many parameters. Quote standardisation is applied consistently throughout.
718-807: LGTM!The multi-line signature reformatting for
post_limit_orderimproves readability given the large number of parameters. All formatting changes are consistent with project standards.
809-869: LGTM!The multi-line signature reformatting for
post_market_orderenhances readability. Quote standardisation is applied consistently throughout the method.
871-950: LGTM!The multi-line signature reformatting for the
sendmethod improves readability for this method with numerous parameters. All formatting is consistent with project standards.
1018-1109: LGTM!The multi-line signature reformatting for the
validatemethod enhances readability given the extensive parameter list. Quote standardisation is applied consistently..pre-commit-config.yaml (1)
1-66: LGTM!The pre-commit configuration is comprehensive and includes appropriate checks for file formatting, Python code quality, linting, security scanning, and modernisation. The hook selections and configurations are sensible.
setup.py (1)
1-18: LGTM!The quote standardisation and f-string modernisation (line 15) improve code consistency and readability. The changes align with project formatting standards.
luno_python/__init__.py (1)
1-1: LGTM!The quote standardisation aligns with the project-wide formatting standard.
tests/test_client.py (3)
14-23: LGTM! Consistent quote standardisation.The quote changes align with the pre-commit configuration (black, isort) and maintain test logic.
26-48: LGTM! Proper error handling tests.The quote standardisation is consistent, and the test correctly verifies that non-JSON responses raise exceptions. The static analysis warning about "blind exception" on line 34 is a false positive—the test intentionally checks that any exception is raised for malformed responses.
51-315: LGTM! Comprehensive test coverage with consistent formatting.The quote standardisation throughout all test functions maintains consistency. The test suite provides excellent coverage for the
get_balances()method, including edge cases like type conversion, empty responses, and malformed data. The static analysis warning about a hardcoded password on line 21 is a false positive—test fixtures with hardcoded credentials are standard practice.luno_python/base_client.py (5)
1-20: LGTM! Improved formatting and quote consistency.The additional blank line on line 3 improves readability between import groups, and the quote standardisation on line 15 aligns with the project's formatting standards.
22-43: LGTM! Improved initialisation pattern.The quote standardisation in the
__init__signature is consistent with the codebase style. The delegation toset_auth,set_base_url, andset_timeoutfollows the DRY principle and provides a cleaner initialisation pattern.
45-61: LGTM! Consistent quote usage.The quote standardisation maintains consistency without altering the logic of these setter methods.
91-99: LGTM! Python 3 modernisation.Replacing
six.iteritems(params)withparams.items()on line 97 is appropriate for Python 3.7+ (as specified in pyproject.toml). This removes the need for the compatibility layer whilst maintaining functionality.
101-105: LGTM! Modern string formatting.The conversion to an f-string on line 105 is more readable and aligns with modern Python practices.
|
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores