Refactor string formatting to use f-strings for improved readability … #67
Conversation
…in multiple files
There was a problem hiding this comment.
Pull request overview
This PR refactors string formatting across multiple Python files to use f-strings instead of string concatenation and the older % or + operators. The changes improve code readability and maintainability. Additionally, several bugs were fixed during the refactoring process, including a missing comma in a list, a typo in an attribute name, and incorrect units in a volume display.
Key changes:
- Converted string concatenation using
+operator to f-strings throughout the codebase - Fixed a critical bug where
inputs.files_fileswas corrected toinputs.files_file - Fixed a missing comma in a list literal that would have caused string concatenation
- Corrected volume units from "cm squared" to "cm^3"
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| r2s_cell.py | Refactored string formatting to f-strings in multiple functions; fixed missing comma in list and typo in attribute name; improved None comparison |
| meshtal_analysis.py | Refactored __str__ method to use f-strings with improved formatting; corrected volume units from cm² to cm³ |
| mcnp_output_reader.py | Updated __str__ methods in multiple tally classes to use f-strings |
| mcnp_input_reader.py | Converted __str__ methods to f-strings across multiple classes; removed debug print statement |
| mcnp_analysis.py | Extensively refactored HTML and CSV output generation to use f-strings; improved code structure with intermediate variables |
| fispact_fluxes_writer.py | Refactored multi-line error message to use f-strings with improved line breaking |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mcnp_analysis.py
Outdated
| if tdat.tally_type == "4": | ||
| hlines.append(f"Number Cells: {len(tdat.cells)}") | ||
| cell_string = "".join(tdat.cells) | ||
| hlines.append(f"Cells: {cell_string}") | ||
| hlines.append(f"Volumes: {''.join(tdat.vols)}") | ||
| # add result plots | ||
| if tdat.eng and len(tdat.cells) == 1: | ||
| pname = "tally_" + str(tdat.number) + \ | ||
| "_" + str(tdat.cells[0]) + ".png" | ||
| title = tdat.particle + \ | ||
| " Spectra for cell " + str(tdat.cells[0]) | ||
| plot_spectra(tdat, pname, title, sp="neutron") | ||
|
|
||
| hlines.append( | ||
| "<img src=" + | ||
| pname + | ||
| " alt=\"simulated spectrum\">") | ||
| if tdat.eng and len(tdat.cells) == 1: | ||
| pname = f"tally_{tdat.number}_{tdat.cells[0]}.png" | ||
| title = f"{tdat.particle} Spectra for cell {tdat.cells[0]}" | ||
| plot_spectra(tdat, pname, title, sp="neutron") | ||
|
|
||
| hlines.append(f"<img src={pname} alt=\"simulated spectrum\">") | ||
| else: | ||
| print("no energy bins or more than one cell") |
There was a problem hiding this comment.
The indentation of this conditional block has been changed. The code for handling tally_type "4" is now nested inside the else block (when statistical test data is not found), which appears to be incorrect. This logic should execute regardless of whether statistical tests are present, so it should be at the same indentation level as the if/else block above it, not nested within the else clause.
r2s_cell.py
Outdated
| # TODO: need to get the groups number for the collapse | ||
| write_collapse(path+"/collapse.i") | ||
| write_array(path+"/array.i") | ||
| write_collapse(f"{path}/collapse.i") |
There was a problem hiding this comment.
Call to function write_collapse with too few arguments; should be no fewer than 2.
| write_collapse(f"{path}/collapse.i") | |
| write_collapse(f"{path}/collapse.i", None) |
…in multiple files