-
Notifications
You must be signed in to change notification settings - Fork 28
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/solve OFAC path issue #351
Conversation
WalkthroughThe pull request introduces version 1.7.1 of the project, which includes a fix for the logic that retrieves the absolute path of the Changes
Poem
TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
CHANGELOG.md (1)
5-7
: LGTM with a minor suggestion.The changelog entry accurately reflects the purpose of the PR and follows the proper versioning convention. Great job on keeping the changelog up-to-date!
Consider simplifying the entry to avoid repetition:
## [1.7.1] - 2024-09-24 ### Fixed -- Fixed logic to get the absolute ofac.json file path ++ Logic to get the absolute ofac.json file pathThis change removes the redundant "Fixed" at the beginning of the line, as it's already implied by the "### Fixed" section header.
Tools
LanguageTool
[duplication] ~6-~6: Possible typo: you repeated a word
Context: ...this file. ## [1.7.1] - 2024-09-24 ### Fixed - Fixed logic to get the absolute ofac.json fil...(ENGLISH_WORD_REPEAT_RULE)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- pyinjective/ofac.py (1 hunks)
- pyproject.toml (1 hunks)
Files skipped from review due to trivial changes (1)
- pyproject.toml
Additional context used
LanguageTool
CHANGELOG.md
[duplication] ~6-~6: Possible typo: you repeated a word
Context: ...this file. ## [1.7.1] - 2024-09-24 ### Fixed - Fixed logic to get the absolute ofac.json fil...(ENGLISH_WORD_REPEAT_RULE)
Ruff
pyinjective/ofac.py
19-21: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
Additional comments not posted (2)
pyinjective/ofac.py (2)
25-25
: Simplified path retrieval looks goodThe updated
get_ofac_list_path
method is a clear improvement. It's more concise and uses a reliable method to get the current file's directory.
15-20
: Improve exception handling and error messageThe new try-except block is a good addition for better error handling. However, there are a couple of improvements we can make:
- Use
raise ... from e
to preserve the original traceback, which aids in debugging.- Make the error message more generic to cover other potential issues.
Here's a suggested improvement:
try: with open(self._ofac_list_path, "r") as f: self._ofac_list = set(json.load(f)) except Exception as e: - raise Exception( - f"Error parsing OFAC list. Please, download it by running python3 pyinjective/ofac_list.py ({e})" - ) + raise Exception( + f"Error reading or parsing OFAC list. If the file is missing, please download it by running python3 pyinjective/ofac_list.py" + ) from eTo ensure the OFAC list file exists and is readable, run the following script:
Summary by CodeRabbit
Release Notes for Version 1.7.1
Bug Fixes
New Features
Documentation