feat: ✨ add CIReport class #1181
Conversation
Update `scipy` version to 1.12.0 for python '>3.8'
github-actions
bot
added
testing
Anselmoo
added
enhancement
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Type: Enhancement
PR Summary: This pull request introduces the CIReport class, aimed at generating and presenting confidence interval reports for a given set of parameters. The class provides functionality to include or exclude offsets in the report, specify the number of digits for numerical values, and print the report as a table. Additionally, the PR includes updates to the PrintingResults class to support custom float formatting and integrates the new CIReport class into the existing codebase, replacing previous functionality with a more structured approach.
Decision: Comment
📝 Type: 'Enhancement' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
- Unsupported files: the diff contains files that Sourcery does not currently support during reviews.
General suggestions:
- Consider improving the clarity of method names within the
CIReportclass to enhance code readability. For instance, renamingconvpto something more descriptive could make the code more accessible to new contributors or maintainers. - Explore the possibility of decoupling report generation from its presentation within the
CIReportclass. This separation of concerns could offer greater flexibility in how reports are generated and presented, potentially accommodating future requirements for different presentation formats or additional data processing before presentation. - Review the use of the
__call__method for generating the report. While this approach is intuitive, providing a more explicitly named method could improve code clarity and maintainability.
spectrafit/report.py: Consider simplifying the addition of the `CIReport` class by directly processing and printing confidence intervals within a single function to reduce complexity.
While the effort to modularize the code and introduce new functionalities is appreciated, the introduction of the CIReport class and its associated methods significantly increases the complexity of the codebase. This added complexity might not necessarily enhance the maintainability or readability of the code. Specifically, the new class and methods introduce additional layers of abstraction, increase the number of decision points, and add more parameters and logic that need to be understood and maintained.
A more streamlined approach could be to enhance the existing functionality without introducing a new class, focusing on simplifying the process of generating and printing the confidence interval report. Here's a suggested simplification that avoids the overhead of class management and method calls while retaining the flexibility of handling offsets and digit rounding:
def print_confidence_interval(ci, with_offset=True, ndigits=5):
"""
Print the confidence intervals in a simplified format.
Args:
ci (Dict[str, List[Tuple[float, float]]]): The confidence intervals for the parameters.
with_offset (bool, optional): Whether to include an offset in the report. Defaults to True.
ndigits (int, optional): The number of digits to round the report values to. Defaults to 5.
"""
df = pd.DataFrame()
for name, intervals in ci.items():
report = {}
offset = 0.0
if with_offset:
offset = next((val for cval, val in intervals if abs(cval) < 1.0e-2), 0.0)
for i, (cval, val) in enumerate(intervals):
sval = val if cval < 1.0e-2 else val - offset
bound_type = "LOWER" if i < len(intervals) / 2 else "UPPER"
report[f"{cval * 100:.2f}% - {bound_type}"] = sval
df[name] = pd.Series(report)
print(df.round(ndigits).to_string())This approach aims to reduce the overall complexity by directly processing and printing the confidence intervals within a single function, thus making the code easier to maintain and understand.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| self.ndigits = ndigits | ||
| self.df = pd.DataFrame() | ||
|
|
||
| def convp(self, x: Tuple[float, float], bound_type: str) -> str: |
There was a problem hiding this comment.
suggestion (llm): The method name convp is not immediately clear in its purpose. Consider renaming it to something more descriptive, such as format_confidence_interval, to improve code readability.
| """Convert the confidence interval to a string.""" | ||
| return "BEST" if abs(x[0]) < 1.0e-2 else f"{x[0] * 100:.2f}% - {bound_type}" | ||
|
|
||
| def __call__(self) -> None: |
There was a problem hiding this comment.
suggestion (llm): Using the __call__ method to generate the report is an interesting choice. It makes the class instance callable, which can be intuitive for users familiar with the pattern. However, for clarity and future maintainability, consider adding a more explicitly named method, like generate, and have __call__ delegate to it. This approach enhances readability and makes the codebase more approachable for new developers or contributors.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1181 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 44 44
Lines 4431 4462 +31
=========================================
+ Hits 4431 4462 +31
|
All PR-Submissions:
Pull Requests for the same
update/change?
New ✨✨ Feature-Submissions:
Changes to ⚙️ Core-Features:
us to include them?