-
Notifications
You must be signed in to change notification settings - Fork 7
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
refactor: ♻️ File I/O operations for better readability and maintainability #1621
Conversation
Review changes with SemanticDiff. Analyzed 9 of 9 files. Overall, the semantic diff is 82% smaller than the GitHub diff.
|
Reviewer's Guide by SourceryThis pull request refactors file I/O operations across multiple files in the spectrafit project to improve readability and maintainability. The main change is the consistent use of the Sequence DiagramsequenceDiagram
participant C as Code
participant P as Path Object
participant F as File
C->>P: Create Path object
C->>P: Call open() method
P->>F: Open file
F-->>C: Return file object
C->>F: Perform I/O operations
C->>F: Close file
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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.
Hey @Anselmoo - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: 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.
@@ -654,7 +654,7 @@ def pkl2any(pkl_fname: Path, encoding: str = "latin1") -> Any: | |||
with gzip.open(pkl_fname, "rb") as f: | |||
return unicode_check(f, encoding=encoding) | |||
elif pkl_fname.suffix == ".pkl": | |||
with open(pkl_fname, "rb") as f: | |||
with pkl_fname.open("rb") as f: |
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.
suggestion: Make pkl2any function consistent with Path.open()
For consistency, consider using Path.open() for all file operations in this function, including the gzip.open() call above.
with pkl_fname.open("rb") as f: | |
with Path(pkl_fname).open("rb") as f: |
@@ -512,8 +512,8 @@ | |||
def save_as_json(self) -> None: | |||
"""Save the fitting result as json file.""" | |||
if self.args["outfile"]: | |||
with open( | |||
Path(f"{self.args['outfile']}_summary.json"), "w", encoding="utf-8" | |||
with Path(f"{self.args['outfile']}_summary.json").open( |
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.
suggestion: Use Path methods for file name manipulation
Instead of using string formatting to create file names, consider using Path methods like with_name() or with_suffix(). For example: Path(self.args['outfile']).with_name(f"{Path(self.args['outfile']).stem}_summary.json")
with Path(f"{self.args['outfile']}_summary.json").open( | |
with Path(self.args['outfile']).with_name(f"{Path(self.args['outfile']).stem}_summary.json").open( |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1621 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 44 44
Lines 4468 4468
=========================================
Hits 4468 4468
Flags with carried forward coverage won't be shown. Click here to find out more.
|
All PR-Submissions:
Pull Requests for the same
update/change?
New ✨✨ Feature-Submissions:
Path
instead ofwith open
#1620Changes to ⚙️ Core-Features:
us to include them?
Summary by Sourcery
Refactor file I/O operations to enhance code readability and maintainability by replacing open() function calls with the Path.open() method across various modules.
Enhancements: