fixing E501 line too long issues and adding restriction to linter#221
fixing E501 line too long issues and adding restriction to linter#221ParticularlyPythonicBS merged 12 commits intounstablefrom
Conversation
ParticularlyPythonicBS
added
the
Maintenance
WalkthroughThis PR systematically addresses E501 line-too-long violations across the entire codebase by breaking long docstrings, error messages, SQL queries, and f-strings across multiple lines. Changes are predominantly formatting-related with no functional modifications, except for three areas: explicit output_comm selection in subqueries (database_util.py), a new SolverNotAvailableError guard (config.py), and super() usage in constructor (mc_worker.py). Changes
Sequence Diagram(s)No sequence diagram needed; the changes are predominantly formatting-related with minimal logic modifications. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
ParticularlyPythonicBS
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
temoa/_internal/exchange_tech_cost_ledger.py (1)
56-60: Region split validation is too weak; malformed links may slip throughThe current check:
r1, r2 = (cast('Region', r) for r in link.split('-')) if not r1 and r2: raise ValueError(f'problem splitting region-region: {link}')only raises when
r1is falsy andr2is truthy. Inputs like"A-"or"A"won’t trigger this guard but will still produce questionable region values, potentially failing later with less clear errors.Consider validating the split more explicitly, e.g.:
- r1, r2 = (cast('Region', r) for r in link.split('-')) - if not r1 and r2: - raise ValueError(f'problem splitting region-region: {link}') + parts = link.split('-') + if len(parts) != 2 or not parts[0] or not parts[1]: + raise ValueError(f'problem splitting region-region: {link}') + r1, r2 = cast('Region', parts[0]), cast('Region', parts[1])This keeps behavior for valid inputs while failing fast and clearly on malformed keys.
temoa/extensions/get_comm_tech.py (1)
12-13: Avoid raising bare strings; use proper Exception typesBoth
get_tperiodsandget_scenariouseraise 'The file type %s is not recognized.' % inp_f, which is invalid in Python 3 (exceptions must derive fromBaseException) and will raise aTypeErrorinstead of the intended message. Refactor to raise an explicit exception type, e.g.:- if not file_ty: - raise 'The file type %s is not recognized.' % inp_f + if not file_ty: + raise ValueError('The file type %s is not recognized.' % inp_f)Apply the same pattern in
get_scenario.Also applies to: 48-49
temoa/components/technology.py (1)
79-95: Fix wording in survival-curve lifetime mismatch warningThe string/docstring reflows look good overall and keep behavior unchanged. There is one likely typo in the warning emitted when
lifetime_process[r, t, v] != p - v:elif value(model.lifetime_process[r, t, v]) != p - v: msg = ( f'The lifetime_process parameter for process ({r, t, v}) with survival ' 'curve ' f'does match the end of that survival curve in {p}. This will waste ' 'compute. To agree with the survival curve and suppress this warning, set ' f'lifetime_process[{r, t, v}] = {p - v}' )This branch handles the non-matching case, so the text should say “does not match”. Suggest:
- f'does match the end of that survival curve in {p}. This will waste ' + f'does not match the end of that survival curve in {p}. This will waste 'Optionally, you may also tweak the
get_default_process_lifetimedocstring to read “This initializer is used to initialize…” for grammar, but that’s cosmetic.Also applies to: 270-331, 334-340, 434-439
temoa/extensions/monte_carlo/mc_run.py (1)
185-192: Fix unpacking ofmodel_dptuple inMCRun.modelpropertyThe
model_dpproperty returns atuple[str, DataPortal](line 178), but line 186 assigns the entire tuple without unpacking. This passes a tuple tocreate_instanceinstead of the expectedDataPortal, breaking instance creation.Other code correctly unpacks this tuple:
run_name, dp = mc_run.model_dp(mc_sequencer.py lines 136, 160) andname, dp = data(mc_worker.py line 123), followed by passing onlydptocreate_instance.Fix at line 186:
@property def model(self) -> TemoaModel: - dp = self.model_dp + _, dp = self.model_dp model = TemoaModel() instance = model.create_instance(data=dp)temoa/extensions/myopic/myopic_sequencer.py (1)
272-281: Parameterize SQL usingscenarioto avoid injection and quoting issuesUnparameterized SQL found at lines 273-281 where
self.config.scenariois directly interpolated. If scenario names contain quotes or SQL metacharacters, this creates an injection vector and quoting bugs:total_cost = self.output_con.execute( 'SELECT SUM(d_invest)+SUM(d_fixed)+SUM(d_var)+SUM(d_emiss) FROM output_cost ' f'WHERE scenario == "{self.config.scenario}"' ).fetchone()[0] ... self.output_con.execute( f"""INSERT INTO output_objective(scenario, objective_name, total_system_cost) VALUES('{self.config.scenario}', 'total_cost', {total_cost})""" )Use parameterized SQL (consistent with patterns elsewhere in this file at line 545-547):
- total_cost = self.output_con.execute( - 'SELECT SUM(d_invest)+SUM(d_fixed)+SUM(d_var)+SUM(d_emiss) FROM output_cost ' - f'WHERE scenario == "{self.config.scenario}"' - ).fetchone()[0] - self.output_con.execute( - f"""INSERT INTO - output_objective(scenario, objective_name, total_system_cost) - VALUES('{self.config.scenario}', 'total_cost', {total_cost})""" - ) + total_cost = self.output_con.execute( + 'SELECT SUM(d_invest)+SUM(d_fixed)+SUM(d_var)+SUM(d_emiss) FROM output_cost ' + 'WHERE scenario == ?', + (self.config.scenario,), + ).fetchone()[0] + self.output_con.execute( + 'INSERT INTO output_objective(scenario, objective_name, total_system_cost) ' + 'VALUES(?, ?, ?)', + (self.config.scenario, 'total_cost', total_cost), + )Also apply the same pattern to the
DELETE FROM output_objectiveat line 265.temoa/components/limits.py (1)
972-987: Minor typo in growth-capacity warning message (missing space)The reformatted warning string:
msg = ( 'Constructing {}rowthCapacity constraint {} and there are period gaps in which' 'capacity cannot exist in this region ({}). Capacity in these periods ' 'will be treated as zero which may cause infeasibility or other problems.' )concatenates to
"…in whichcapacity cannot exist…", missing a space between"which"and"capacity".Consider adding a trailing space to the first literal (or a leading space to the second):
- 'Constructing {}rowthCapacity constraint {} and there are period gaps in which' - 'capacity cannot exist in this region ({}). Capacity in these periods ' + 'Constructing {}rowthCapacity constraint {} and there are period gaps in which ' + 'capacity cannot exist in this region ({}). Capacity in these periods 'temoa/components/costs.py (1)
491-507: End-of-life emissions cost uses consistent pattern; minor comment nit
endoflife_emissionsstill treats end-of-life emissions as a fixed cost over the retirement period, withcost_years=value(model.period_length[p])tied to the current period and a guard onretirement_periods. Only comment wrapping changed, though the comment still refers to “embodied emissions”, which may be confusing; consider updating it to “end-of-life emissions” for accuracy.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (61)
.github/scripts/deploy_nightly_pypi.py(1 hunks)pyproject.toml(1 hunks)temoa/_internal/data_brick.py(2 hunks)temoa/_internal/exchange_tech_cost_ledger.py(3 hunks)temoa/_internal/run_actions.py(4 hunks)temoa/_internal/table_data_puller.py(4 hunks)temoa/_internal/table_writer.py(6 hunks)temoa/_internal/temoa_sequencer.py(3 hunks)temoa/cli.py(6 hunks)temoa/components/capacity.py(6 hunks)temoa/components/commodities.py(2 hunks)temoa/components/costs.py(9 hunks)temoa/components/geography.py(1 hunks)temoa/components/limits.py(13 hunks)temoa/components/operations.py(7 hunks)temoa/components/reserves.py(2 hunks)temoa/components/storage.py(2 hunks)temoa/components/technology.py(5 hunks)temoa/components/time.py(5 hunks)temoa/core/config.py(2 hunks)temoa/core/model.py(4 hunks)temoa/data_io/hybrid_loader.py(9 hunks)temoa/data_processing/db_query.py(1 hunks)temoa/data_processing/make_output_plots.py(1 hunks)temoa/extensions/breakeven/breakeven.py(4 hunks)temoa/extensions/get_comm_tech.py(9 hunks)temoa/extensions/method_of_morris/morris.py(3 hunks)temoa/extensions/method_of_morris/morris_evaluate.py(2 hunks)temoa/extensions/method_of_morris/morris_sequencer.py(7 hunks)temoa/extensions/modeling_to_generate_alternatives/hull.py(1 hunks)temoa/extensions/modeling_to_generate_alternatives/mga_sequencer.py(2 hunks)temoa/extensions/modeling_to_generate_alternatives/tech_activity_vector_manager.py(3 hunks)temoa/extensions/monte_carlo/example_builds/scenario_maker.py(2 hunks)temoa/extensions/monte_carlo/mc_run.py(5 hunks)temoa/extensions/monte_carlo/mc_sequencer.py(2 hunks)temoa/extensions/monte_carlo/mc_worker.py(2 hunks)temoa/extensions/myopic/myopic_progress_mapper.py(1 hunks)temoa/extensions/myopic/myopic_sequencer.py(9 hunks)temoa/extensions/single_vector_mga/sv_mga_sequencer.py(2 hunks)temoa/extensions/stochastics/VSS.py(5 hunks)temoa/model_checking/commodity_network.py(4 hunks)temoa/model_checking/network_model_data.py(2 hunks)temoa/model_checking/pricing_check.py(4 hunks)temoa/model_checking/validators.py(6 hunks)temoa/types/validation_types.py(1 hunks)temoa/utilities/database_util.py(4 hunks)temoa/utilities/db_migration_to_v3.py(4 hunks)temoa/utilities/db_migration_v3_to_v3_1.py(6 hunks)temoa/utilities/graphviz_generator.py(1 hunks)temoa/utilities/sql_migration_v3_1_to_v4.py(3 hunks)temoa/utilities/visualizer.py(1 hunks)tests/conftest.py(1 hunks)tests/test_cli.py(2 hunks)tests/test_emission_results.py(8 hunks)tests/test_full_runs.py(1 hunks)tests/test_linked_tech.py(1 hunks)tests/test_material_results.py(1 hunks)tests/test_set_consistency.py(1 hunks)tests/test_storage.py(1 hunks)tests/test_tech_activity_vectors.py(1 hunks)tests/utilities/capture_set_sizes_for_cache.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-03T22:55:02.519Z
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 186
File: temoa/_internal/temoa_sequencer.py:60-63
Timestamp: 2025-11-03T22:55:02.519Z
Learning: In `TemoaSequencer.__init__`, when a `mode_override` is provided, it must be written back to `self.config.scenario_mode` (not just stored in `self.temoa_mode`) because `HybridLoader.create_data_dict()` has strict consistency checks that validate `config.scenario_mode` matches the presence/absence of `myopic_index`. Without this synchronization, MYOPIC mode would fail with a RuntimeError when the loader is instantiated.
Applied to files:
temoa/components/time.pytemoa/_internal/temoa_sequencer.py
📚 Learning: 2025-10-30T14:37:44.260Z
Learnt from: idelder
Repo: TemoaProject/temoa PR: 180
File: temoa/components/limits.py:632-647
Timestamp: 2025-10-30T14:37:44.260Z
Learning: In the `limit_tech_input_split_average_constraint` function in `temoa/components/limits.py`, there was a bug where the `total_inp` summation incorrectly used the fixed variable `i` instead of the loop variable `S_i` when indexing `processOutputsByInput` and calling `get_variable_efficiency`. This pattern should be watched for in similar aggregation constraints: when summing over a loop variable like `S_i`, all references within that loop should use `S_i`, not a fixed outer variable.
Applied to files:
temoa/components/limits.py
🧬 Code graph analysis (3)
temoa/extensions/myopic/myopic_sequencer.py (2)
temoa/core/model.py (1)
TemoaModel(93-1137)temoa/types/model_types.py (1)
TemoaModel(148-224)
tests/test_emission_results.py (1)
tests/test_material_results.py (1)
solved_connection(16-43)
temoa/components/limits.py (1)
temoa/components/geography.py (1)
gather_group_regions(36-44)
🪛 Ruff (0.14.5)
temoa/utilities/database_util.py
201-202: Possible SQL injection vector through string-based query construction
(S608)
252-253: Possible SQL injection vector through string-based query construction
(S608)
temoa/utilities/sql_migration_v3_1_to_v4.py
259-260: Possible SQL injection vector through string-based query construction
(S608)
temoa/extensions/get_comm_tech.py
33-33: Possible SQL injection vector through string-based query construction
(S608)
temoa/extensions/breakeven/breakeven.py
428-439: Use f-string instead of format call
Convert to f-string
(UP032)
437-437: value may be undefined, or defined from star imports
(F405)
temoa/extensions/monte_carlo/mc_run.py
143-143: Avoid specifying long messages outside the exception class
(TRY003)
temoa/utilities/db_migration_v3_to_v3_1.py
297-298: Possible SQL injection vector through string-based query construction
(S608)
375-375: Possible SQL injection vector through string-based query construction
(S608)
temoa/extensions/myopic/myopic_sequencer.py
274-275: Possible SQL injection vector through string-based query construction
(S608)
tests/test_emission_results.py
89-90: Possible SQL injection vector through string-based query construction
(S608)
116-117: Possible SQL injection vector through string-based query construction
(S608)
144-145: Possible SQL injection vector through string-based query construction
(S608)
171-172: Possible SQL injection vector through string-based query construction
(S608)
254-255: Possible SQL injection vector through string-based query construction
(S608)
343-344: Possible SQL injection vector through string-based query construction
(S608)
🔇 Additional comments (112)
temoa/extensions/modeling_to_generate_alternatives/tech_activity_vector_manager.py (3)
222-223: Docstring correctly split for E501 compliance.The multi-line docstring reformatting maintains semantic correctness and readability while complying with the 100-character line limit.
266-267: Docstring correctly split for E501 compliance.The multi-line docstring reformatting maintains semantic correctness and readability while complying with the 100-character line limit.
304-305: Comment correctly split for E501 compliance.The multi-line comment split maintains semantic correctness while complying with the 100-character line limit.
temoa/model_checking/network_model_data.py (1)
269-270: SQL JOIN condition formatting is correct for E501 compliance.Both LEFT JOIN ON conditions have been cleanly split across lines while preserving the join logic (
eff.tech = lp.tech AND eff.vintage = lp.vintage AND eff.region = lp.region). The changes are consistent across the two query branches (with/without sector), and the multi-line ON clauses are valid SQL that SQLite will parse correctly.Please verify that each split line is now ≤100 characters to confirm E501 compliance.
Also applies to: 281-282
temoa/extensions/modeling_to_generate_alternatives/hull.py (1)
97-100: The refactored comments comply with the line length requirement.Verification confirms all changed lines are within the 100-character E501 limit:
- Line 97: 95 characters ✓
- Line 99: 97 characters ✓
- Line 111: 100 characters ✓
- Line 112: 23 characters ✓
The code changes successfully address E501 violations.
temoa/components/reserves.py (2)
183-200: Docstring reformatting preserves mathematical content; verify rendering.The LaTeX math expressions have been successfully split across lines to meet the 100-character limit. Since LaTeX treats whitespace in source as insignificant, the rendered equations should be identical to the original. However, verify that the reformatted docstring renders correctly in Sphinx/documentation builds, particularly for the math directive output.
265-270: Error message reformatting is correct; implicit string concatenation works as intended.The multi-line error message using implicit string concatenation (f-string + regular string) will produce the same output as the original long line. The split maintains the exact error text and is properly formatted.
temoa/data_processing/db_query.py (4)
3-8: Module docstring formatting looks good.The docstring is properly split across multiple lines and appears compliant with the 100-character limit. The formatting change is consistent with the PR objectives.
18-50: Function implementations look compliant.The
send_query()function and its error handling appear to maintain lines within the 100-character limit. No issues detected in this section.
52-105: Remaining functions are compliant.The
help_user(),get_flags(), and__main__block all maintain lines within the 100-character limit. The formatting is clean and the changes address E501 violations appropriately.
1-2: No issues found—code is compliant with E501 line length limits.Line 1 is 94 characters (within the 100-character limit) and line 2 is 11 characters. The header comment formatting change correctly addresses the E501 violation.
temoa/components/commodities.py (4)
228-232: Docstring equation reflow looks good.The LaTeX math expressions in the demand_activity_constraint docstring have been split appropriately at operator boundaries to comply with the 100-character line limit. The mathematical notation and meaning are preserved.
321-322: Description text wrapping is appropriate.The commodity balance description has been split across two lines at a natural logical break, improving readability while preserving the meaning.
330-337: Multi-term equation reflow is consistent and correct.The commodity balance equation terms (annual processes, inter-regional imports, end-of-life outputs) have been split appropriately, with breaks occurring after operators and before fractions. The aligned environment structure is maintained.
344-356: Consumption-side equation terms are properly reflowed.The consumption-side terms (non-storage processes, annual processes, inter-regional exports, construction inputs) have been split consistently across lines, maintaining the aligned environment structure and mathematical correctness. All breaks occur at logical operator boundaries.
temoa/_internal/temoa_sequencer.py (3)
142-143: LGTM! Error message correctly split for line length compliance.The error message is properly split across two lines using implicit string concatenation. The spacing is preserved correctly and the resulting message is identical to the original.
159-160: LGTM! Error message correctly split for line length compliance.The error message is properly split across two lines using implicit string concatenation. The spacing is preserved correctly and the resulting message is identical to the original.
233-234: LGTM! Error message correctly split for line length compliance.The error message is properly split across two lines using implicit string concatenation. The spacing is preserved correctly and the resulting message is identical to the original.
temoa/_internal/exchange_tech_cost_ledger.py (3)
3-5: Docstring reflow is safe and aligns with E501 goalRewrapping the module docstring to multiple lines keeps content intact and meets the line-length requirement without changing behavior.
74-76: Comment wrapping aroundTemoaModelcast is correct and clearerSplitting the explanatory comment over two lines improves readability and E501 compliance without affecting the
castusage or runtime behavior.
134-135: Loop comment reflow is non-functional and matches linting objectiveReflowing the comment that describes iterating region pairings keeps the meaning intact and satisfies the line-length constraint.
tests/test_tech_activity_vectors.py (1)
23-24: Comment reflow maintains clarity and fixes E501The wrapped comment still clearly communicates the mapping purpose, with no impact on test behavior.
temoa/utilities/db_migration_to_v3.py (3)
174-181: Warning message wrapping is semantically unchangedSplitting the long warning string into adjacent literals keeps the exact message content while satisfying E501; no behavioral change.
216-218: RPS group comparison message rewrapped correctlyThe multi-line string for the RPS group comparison log reads cleanly and preserves the original wording.
306-307: Comment wrapping around tech_groups rename is fineThe reflowed comment still accurately explains the region–group “smashing” behavior; formatting-only change.
tests/conftest.py (1)
37-40: Docstring reflow forrefresh_databaseslooks goodThe new multi-line docstring clearly describes the fixture setup behavior while resolving the long-line issue.
temoa/extensions/method_of_morris/morris.py (1)
79-81: SQL query splitting preserves semanticsBreaking these long SELECT statements into adjacent string literals (with trailing spaces) keeps the SQL identical while satisfying E501; no functional change to the Method of Morris workflow.
Also applies to: 100-103, 118-121
tests/test_material_results.py (1)
69-70: Expanded comment clarifies None-handlingThe updated comment clearly distinguishes between
row is Noneandrow[0] is None, matching the defensiveflowassignment below.temoa/utilities/visualizer.py (1)
230-232: Exception message wrapping is safe and readableThe FileNotFoundError log message is simply rewrapped into adjacent literals; content and logging behavior remain the same.
tests/test_full_runs.py (1)
69-70: Domain-size comment reflow is fineThe wrapped comment still correctly notes the potential expense of building the domain for large models; formatting-only change.
temoa/_internal/data_brick.py (1)
2-3: LGTM! Docstring formatting improvements.The docstring wrapping successfully addresses E501 line length violations without affecting functionality.
Also applies to: 25-26
temoa/extensions/modeling_to_generate_alternatives/mga_sequencer.py (2)
37-38: LGTM! Path assignment formatting improvement.The file path assignment is appropriately split across multiple lines to comply with line length limits.
339-340: LGTM! Error message formatting improvement.The error message is appropriately split across lines while maintaining readability.
temoa/extensions/monte_carlo/mc_worker.py (2)
31-33: LGTM! Comment formatting improvement.The docstring comment is appropriately wrapped to comply with line length requirements.
65-65: Good refactor to usesuper().While the PR focuses on E501 formatting fixes, this change from explicit parent class reference to
super()is a positive improvement that follows Python best practices. The behavior remains identical.temoa/components/capacity.py (5)
68-70: LGTM! Log message formatting improvement.The warning message is appropriately split across multiple lines for better readability and E501 compliance.
122-123: LGTM! Docstring formatting improvement.The docstring is appropriately wrapped to comply with line length requirements.
295-296: LGTM! Mathematical formula and comment formatting improvements.The LaTeX formula and code comment are appropriately split across lines while maintaining readability.
Also applies to: 335-336
503-504: LGTM! Docstring formatting improvements.The figure caption and related docstring text are appropriately wrapped to comply with line length limits.
Also applies to: 508-509
601-602: LGTM! List item formatting improvement.The docstring list item is appropriately wrapped while maintaining clarity.
tests/test_linked_tech.py (1)
49-50: LGTM! Comment formatting improvement.The test comment is appropriately wrapped to comply with line length requirements without affecting test functionality.
tests/utilities/capture_set_sizes_for_cache.py (1)
18-21: LGTM! Warning message formatting improvement.The warning message is appropriately split across multiple lines to comply with line length limits while maintaining clarity.
.github/scripts/deploy_nightly_pypi.py (1)
175-176: LGTM! Print statement formatting improvement.The print statement is appropriately split using f-string concatenation to comply with line length requirements.
temoa/types/validation_types.py (1)
43-44: LGTM! Docstring formatting improvement.The attribute documentation is appropriately wrapped to comply with line length requirements while maintaining clarity.
tests/test_storage.py (1)
88-91: Storage sequencing assertion message split is safeThe assertion’s error message is now split across two adjacent f-strings with a trailing space; Python concatenation preserves the exact text and behavior.
temoa/extensions/monte_carlo/example_builds/scenario_maker.py (1)
9-16: Docstring and CSV string wrapping preserve behaviorDocstring reflow and splitting of CSV format strings into adjacent literals keep the generated text and file contents identical while satisfying E501.
Also applies to: 53-55, 60-61
tests/test_set_consistency.py (1)
66-68: Comment reflow only; set filtering logic unchangedThe comment explaining why
_indexsets are dropped is just rewrapped; the comprehension that filtersmissing_in_modelis untouched and remains correct.temoa/extensions/get_comm_tech.py (1)
22-23: Cursor and explanatory comments rewrapped cleanlyThe long inline comments around cursor usage, efficiency parsing, and DB state are split across lines without changing code execution or meaning; E501 is satisfied with no behavioral impact.
Also applies to: 58-59, 80-81, 114-115, 148-149, 177-178, 220-221
temoa/components/geography.py (1)
136-142: Error message wrapping preserves clarity and behaviorThe invalid-region message is now built via a multi-line
msgvariable combining an f-string and a literal; the final text is unchanged and still logged and raised as before, while satisfying E501.temoa/data_processing/make_output_plots.py (1)
236-236: LGTM: Comment formatting adjustment.The shortened separator comment correctly addresses the line length requirement with no functional impact.
temoa/extensions/single_vector_mga/sv_mga_sequencer.py (2)
154-157: LGTM: Proper multi-line string formatting.The error message has been correctly split across multiple lines while preserving the exact message content.
247-255: LGTM: Warning message properly reformatted.The multi-line warning message correctly maintains semantic equivalence while addressing line length constraints.
tests/test_emission_results.py (2)
89-90: LGTM: SQL query strings properly reformatted.The SQL queries have been correctly split across multiple lines to address line length requirements. The concatenated f-strings produce identical queries to the original code.
Note: Static analysis flags potential SQL injection (S608) at these locations, but this is a pre-existing pattern. In this test context, the
techvariable comes from controlled test parameters, so it's not a security concern.Also applies to: 116-117, 144-145, 171-172, 254-255, 343-344
164-166: LGTM: Docstrings and assertion messages properly wrapped.The multi-line formatting of docstrings and assertion messages correctly addresses line length constraints with no functional changes.
Also applies to: 204-206, 247-249, 287-289
temoa/_internal/table_data_puller.py (1)
1-8: LGTM: Documentation formatting improvements.The module docstring, inline comments, and parameter descriptions have been properly reflowed to comply with line length requirements. No functional changes.
Also applies to: 80-81, 282-283, 574-575
temoa/_internal/run_actions.py (2)
78-84: LGTM: Error messages properly reformatted.The database version error messages have been correctly split across multiple lines while maintaining the exact error text.
Also applies to: 93-94
248-251: LGTM: Comments and docstrings properly wrapped.The dev note and docstring return description have been appropriately reflowed to meet line length requirements.
Also applies to: 308-309
temoa/extensions/myopic/myopic_progress_mapper.py (1)
75-78: LGTM: Timestamp formatting improved with label.The timestamp now returns a labeled format ("Elapsed: HH:MM:SS ") using multi-line f-string formatting. This enhances readability while addressing line length requirements. The time calculation logic remains unchanged.
temoa/extensions/method_of_morris/morris_evaluate.py (1)
27-28: LGTM: Module docstring and error message properly formatted.The module docstring and runtime error message have been correctly split across multiple lines to comply with line length requirements, preserving the exact text content.
Also applies to: 125-126
temoa/utilities/graphviz_generator.py (1)
264-267: LGTM: Format string properly wrapped.The
tech_attr_fmtstring has been correctly split across multiple lines using tuple concatenation. The label, capacity, href, and onclick attributes remain functionally identical.pyproject.toml (1)
108-110: LGTM! Good improvement to linting configuration.The change from a global E501 ignore to a per-file ignore scoped only to legacy files is an excellent improvement. This ensures that line-too-long violations are caught everywhere except for legacy code that may be harder to refactor.
temoa/_internal/table_writer.py (1)
48-50: LGTM! Clean formatting improvements.All changes in this file are cosmetic line-wrapping to comply with the 100-character limit. Docstrings, comments, and warning messages have been properly reformatted without altering their content or functionality.
Also applies to: 101-108, 205-207, 389-391, 509-510, 535-537
temoa/components/operations.py (1)
295-296: LGTM! Proper line-wrapping implementation.All changes correctly reformat docstrings and warning messages to comply with line length limits. The warning messages use implicit string concatenation (adjacent string literals), which is idiomatic Python and maintains the original message content.
Also applies to: 340-341, 430-431, 458-459, 505-506, 533-534, 580-581
temoa/model_checking/commodity_network.py (1)
156-157: LGTM! Consistent logging message formatting.All changes properly reformat logging messages to meet line length requirements using implicit string concatenation. The functionality and message content remain unchanged.
Also applies to: 165-166, 212-213, 285-286, 296-297
temoa/extensions/method_of_morris/morris_sequencer.py (1)
2-3: LGTM! Well-formatted changes.All changes are cosmetic line-wrapping improvements for docstrings, comments, messages, and SQL queries. The SQL query reformatting maintains the original semantics while improving readability.
Also applies to: 54-55, 109-110, 124-128, 133-134, 147-148, 279-280, 299-300
temoa/extensions/breakeven/breakeven.py (1)
247-248: LGTM! Formatting improvements applied.All changes properly reformat code (including commented code) to comply with line length limits. The multi-line string concatenation at lines 427-440 is correctly implemented.
Also applies to: 427-440, 1016-1022, 1122-1124
temoa/extensions/stochastics/VSS.py (1)
96-102: LGTM! Improved readability.All changes are cosmetic line-wrapping improvements. The tuple construction at lines 96-102 is particularly well-formatted, making the multi-element key more readable.
Also applies to: 155-157, 167-173, 240-242, 263-266
temoa/utilities/database_util.py (3)
141-144: LGTM! Clean formatting.The WHERE clause is properly reformatted across multiple lines for readability without changing functionality.
200-203: LGTM! Query formatting improved.The query strings are properly reformatted using parentheses for multi-line readability. The SQL injection warnings from static analysis are false positives in this context as
self.scenariois a controlled instance attribute.Also applies to: 251-254
309-310: Verified: Functional change is correct and necessary.Lines 309-310 and 315-316 add
output_commto the SELECT clauses of both subqueries (OF and OFO). Verification confirms this is a functional fix, not a formatting change.Analysis:
- The outer query references
OF.output_commin SELECT, JOIN, and GROUP BY conditions- SQL requires all columns referenced by an outer query to be explicitly selected in the subquery
- Without these additions, the query would fail with a column-not-found error
Verification Results:
- The function is actively called from
graphviz_generator.py:377during diagram generation- This code path is tested by
test_full_runs.py::test_graphviz_integration, which runs a full model with graphviz output enabled- No issues or breakage detected; the change correctly fixes the SQL query
temoa/model_checking/pricing_check.py (1)
182-188: Logging and docstring wrapping looks correctThe reflowed warning/error strings and docstring bullets keep the original wording and spacing while satisfying the line-length limit. No behavioral or formatting regressions detected in these checks.
Also applies to: 232-239, 243-250, 264-287, 313-326
temoa/extensions/myopic/myopic_sequencer.py (1)
125-137: Myopic sequencer logging/comment wrapping is safeThe reflowed multi-line log/error messages and comments keep their original meaning and spacing, and the rewrapped DELETE statement still uses placeholders correctly. No behavioral impact from these E501 fixes.
Also applies to: 184-196, 247-249, 366-368, 411-421, 433-440, 555-557
temoa/core/model.py (1)
125-159: Reflowed comments/docstrings clarify model internals without changing behaviorAll modifications here are comment/docstring repositioning and small wording tweaks around the sparse dictionaries, time sequencing, switching sets, and capacity-factor defaults. The underlying attributes and Pyomo constructs are unchanged, so behavior stays identical while readability improves.
Also applies to: 174-195, 275-285, 381-382, 441-448, 450-488, 491-507, 529-533
temoa/data_io/hybrid_loader.py (1)
209-212: HybridLoader SQL and logging reflow preserves behaviorThe rewrapped SQL (
time_periodqueries, myopicexisting_capacityUNION, mainefficiencySELECT) and the adjusted warning/error messages all preserve their original semantics and spacing, while complying with the line-length limit. Parameters continue to be passed via placeholders where appropriate.Also applies to: 237-239, 383-385, 449-451, 635-637, 704-706, 746-748, 796-798, 905-907, 909-911, 915-917, 921-922
temoa/extensions/monte_carlo/mc_run.py (1)
44-48: Tweak__repr__/ logging comment changes are fineThe updated
__repr__formatting and the comment about skipping runs with no successful tweaks are purely cosmetic and keep the previous behavior and intent.Also applies to: 353-355
temoa/extensions/monte_carlo/mc_sequencer.py (1)
98-105: MCSequencer queue/comment and error-message wrapping are safeThe queue initialization is unchanged aside from the explanatory comment reflow, and the
ValueErrorinprocess_solve_resultsnow uses split literals with correct spacing. No impact on runtime behavior.Also applies to: 269-271
temoa/components/limits.py (9)
52-57: Input-split annual warning reflow preserves semanticsThe warning message split across multiple string literals keeps the original content and formatting while resolving line-length issues; no behavioral change here.
103-107: Output-split annual warning reflow is consistent with input-splitSame pattern as the input-split warning: content, formatting, and
.format(ann_indices)usage are intact; only E501-related wrapping.
258-262: Dev-note clarification onlimit_resourceis accurateThe added comments clarify that this is effectively a global activity constraint (often on dummy-import techs), matching the implementation that sums activity over all periods and regions; no logic change.
307-314: Math docstring reflow forlimit_activity_shareremains mathematically correctThe LaTeX formulation is merely wrapped onto shorter lines; the index structure and inequality are unchanged and still match the implementation below.
457-468: Annual capacity factor comments correctly describe region-group behaviorThe added comments about
rbeing a single region, region group (Mexico+US+Canada), or'global'line up withgeography.gather_group_regionsusage and help future readers; logic is unmodified.
530-538: Seasonal capacity factor comments mirror annual constraint correctlyThe explanatory comments for
limit_seasonal_capacity_factor_constraintuse the same region semantics as the annual version and match howgather_group_regionsand capacity checks are used; no functional impact.
645-663:limit_tech_input_split_average_constraintindices and loop vars look correct
inpandtotal_inpboth iterate overS_s,S_d, andS_i, and consistently use those loop variables when indexingprocess_outputs_by_inputand callingget_variable_efficiency.- Using
model.limit_tech_input_split_annual[...]as the parameter for average constraints is consistent with the database mapping forTechInputSplitAverage→LimitTechInputSplitAnnualseen in the migration script and avoids introducing a separate parameter.This addresses the prior class of bugs where a fixed outer variable was mistakenly used instead of the loop variable.
Based on learnings
828-887: Emission constraint comments clarify exclusion of flex/curtailmentThe new comments explain why flex and curtailment emissions are intentionally not double-counted given they are already handled via
v_flow_out/v_flow_out_annual. The underlying sums (process_emissions,_annual,embodied_emissions,retirement_emissions) are unchanged and remain consistent with the description.
1208-1233: Growth-new-capacity-delta warnings look correct and readableThe long warning messages for missing periods and gaps in
limit_growth_new_capacity_deltaare cleanly split across literals with appropriate spaces preserved; semantics and logging behavior are unchanged.temoa/core/config.py (2)
12-32: CustomSolverNotAvailableErrorandSOLVER_DOC_LINKSlook appropriateDefining a dedicated
SolverNotAvailableErrorand centralizing solver-installation URLs inSOLVER_DOC_LINKSis a good step for clearer error handling. The keys (cbc,gurobi,cplex,highs,glpk) and messages are sensible and keep line lengths within bounds.
172-193: All repository config files already specifysolver_name; verify impact on external configurationsThe codebase contains no legacy config files without
solver_name. Both the tutorial sample and all test configs explicitly specifysolver_name = "cbc". Tests confirm the new behavior validates solver availability and raiseSolverNotAvailableErrorwhen missing. However, no explicit documentation confirms whether this was already a required field or represents a breaking change from prior versions, and external user configurations are not visible in the repository. Ensure any published migration guidance addresses this requirement.temoa/utilities/db_migration_v3_to_v3_1.py (3)
70-75:column_checkerror message reflow keeps intent and improves readabilityThe multi-line construction of the “Columns … missing from …” message maintains the same content and adds a clear “Try adding or renaming…” clause, with proper spacing preserved across literals.
186-188:new_colscomprehension rewrap is cosmetic onlyThe reflow of the
new_colslist comprehension over multiple lines does not change semantics; it still reflects the column order fromPRAGMA table_info.
297-299: Use parameterized queries instead of string interpolation forregion/techfiltersBoth of these queries build SQL by interpolating
regionandtechvalues directly into the string:con_old.execute( f'SELECT vintage FROM Efficiency WHERE region == "{r}" AND ' f'tech == "{t}"' ) con_old.execute( f'SELECT vintage FROM Efficiency WHERE region=="{row[0]}" AND ' f'tech="{row[1]}"' )Even though these values originate from existing database content, they can still contain problematic characters and pose an injection / quoting risk, and Ruff correctly flags them (S608).
You can avoid this by using SQLite parameters for the WHERE-clause values:
- t[0] for t in con_old.execute( - f'SELECT vintage FROM Efficiency WHERE region == "{r}" AND ' - f'tech == "{t}"' - ).fetchall() + t[0] + for t in con_old.execute( + 'SELECT vintage FROM Efficiency WHERE region == ? AND tech == ?', + (r, t), + ).fetchall()and similarly in the
LoanLifetimeTechloop:- vints = [ - v[0] - for v in con_old.execute( - f'SELECT vintage FROM Efficiency WHERE region=="{row[0]}" AND ' - f'tech="{row[1]}"' - ).fetchall() - ] + vints = [ + v[0] + for v in con_old.execute( + 'SELECT vintage FROM Efficiency WHERE region == ? AND tech == ?', + (row[0], row[1]), + ).fetchall() + ]This keeps logic identical while eliminating the injection vector and escaping concerns.
Also applies to: 372-377
⛔ Skipped due to learnings
Learnt from: ParticularlyPythonicBS Repo: TemoaProject/temoa PR: 184 File: temoa/data_processing/database_util.py:109-118 Timestamp: 2025-11-03T13:34:35.907Z Learning: In the Temoa project, SQL injection vulnerabilities in database utility classes (such as temoa/data_processing/database_util.py) are considered low concern due to the data access patterns and should not be flagged during code reviews. Technical debt for SQL injection hardening should be tracked separately as issues rather than blocking PRs.Learnt from: ParticularlyPythonicBS Repo: TemoaProject/temoa PR: 184 File: temoa/data_processing/database_util.py:109-118 Timestamp: 2025-11-03T13:34:35.907Z Learning: In the Temoa project, SQL injection vulnerabilities in the database utility classes (such as temoa/data_processing/database_util.py) are considered low concern due to the data access patterns and should not be flagged during reviews. Technical debt for SQL injection hardening should be tracked separately as issues rather than blocking PRs.temoa/components/time.py (5)
101-112:validate_segment_fractionmismatch message is now clearerThe reworked
msgfor mismatchedtime_segment_fractionindices gives separated sections for missing vs extra indices, with correct newlines and spacing; logic aroundexpected_keys/keysand the subsequentValueErroris unchanged.
165-174: Manual sequencing validation message formatting improvedThe message raised when manual sequencing leaves states without successors or predecessors is now split into two clearly-labelled sections. The underlying set logic (
missing_psd,missing_psd_next) and exception type remain the same.
296-303: Invalidtime_sequencingerror preserves behavior with better contextThe default branch logs and raises a
ValueErrorwith a more explicit message including the offendingtime_sequencingvalue and hinting at a likely code error; control flow is unchanged.
351-360:time_season_sequentialmissing-data message is more informativeThe error when
time_season_sequentialis empty but inter-season features are used now clearly references the currenttime_sequencingmode and explains thattime_season_sequentialmust be filled whenever seasonal storage or ramping are enabled; behavior (log +ValueError) is unchanged.
371-379: Consecutive-days discontinuity warning text is correctThe newly wrapped warning for unequal
num_daysin consecutive seasons underconsecutive_dayssequencing preserves the prior logic and clearly explains why this is problematic; numerical checks and exception type remain the same.temoa/model_checking/validators.py (4)
79-86: Linked-tech missing vintage error message reflow is soundThe error log for missing driven-tech vintages still reports
t_driven,v, andt_driverin order, with the message split across literals only for line length; the exception path (return False) is unchanged.
88-96: Linked-tech lifetime mismatch message remains informativeThe lifetime mismatch error keeps all dynamic fields (driven tech, its lifetime, vintage, driver tech, driver lifetime) and now reads more cleanly across lines; the control flow (log, print,
return False) is intact.
343-350: Reserve-margin warning text correctly explains missing reserve techsThe wrapped warning in
validate_reserve_marginstill explains that a planning reserve margin was provided but no reserve technologies serve that region, and logs both the region and margin value. Only the string formatting changed.
373-381:check_no_intersectionnow logs intersecting elements explicitlyThe new message clearly reports both sets and the list of violating members. This is helpful when debugging forbidden intersections, and the boolean contract of
check_no_intersectionis unchanged.temoa/components/costs.py (3)
145-156:loan_costdocstring reflow improves clarity without code changesThe docstring now more clearly explains that the function works both with fixed values and Pyomo objects; parameters and return semantics are unchanged. Implementation of the annuity and discounting pipeline is intact.
472-489: Embodied emissions cost comment is clear; use ofperiod_length[v]is safe
- The comment now explicitly states the assumption that embodied emissions are emitted in the same year capacity is installed.
- Using
cost_years=value(model.period_length[v])is equivalent toperiod_length[p]under theif v == pguard in the comprehension, so there is no behavioral change.
523-662: Objective function documentation reflow is comprehensive and consistent with implementationThe large docstring for
total_cost_rulehas been rewrapped to meet line-length constraints but still accurately documents the multi-step discounting for loans (with and without survival curves) and the treatment of fixed/variable/emission costs. The actual objective expression remainsquicksum(period_cost_rule(model, p) for p in model.time_optimize), unchanged.tests/test_cli.py (3)
408-444: Tutorial CBC-availability tests align with new solver guidanceThe tests simulate CBC presence/absence via
shutil.whichand_check_cbc_availabilityand assert that the tutorial command both completes successfully and reports the expected warning (including the Cbc download URL) only when CBC is missing. This nicely guards the user messaging introduced elsewhere without changing behavior in other cases.
488-516:validateCLI test correctly exercisesSolverNotAvailableErrorpath
test_cli_validate_fails_if_solver_missing:
- Forces a nonexistent solver in the config and patches
shutil.whichto always returnNone, ensuring the new solver-availability logic is hit.- Asserts
SystemExit(1)and that the CLI prints a clear “❌ Validation failed:” message along with the guidance “Please ensure the solver is installed and its executable is accessible.”This accurately captures the expected behavior from the new
build_configerror handling.
518-543:runCLI test mirrors solver-missing behavior and messaging
test_cli_run_fails_if_solver_missingmirrors the validate test for theruncommand, ensuring that a missing solver triggersSystemExit(1), an “❌ An error occurred:” message, and the same installation guidance text. This provides good coverage for both primary CLI entry points using the new solver checks.temoa/cli.py (7)
127-129: LGTM — String literal concatenation correctly applied.The citation text is properly split across three adjacent string literals using implicit concatenation. This preserves the original content while complying with the 100-character line limit.
157-158: LGTM — Comment wrapping is clean and maintains readability.The fallback-path explanation is split logically across two comment lines without losing context or clarity.
327-328: LGTM — Help text split correctly using adjacent string literals.The migration-type help parameter is properly split across two lines. The implicit concatenation preserves the original help text without any unintended whitespace artifacts.
396-397: LGTM — Error message formatted correctly with f-string concatenation.Two f-strings are properly chained to format the error message while staying within the line-length limit.
599-600: LGTM — Tutorial guidance message properly split with f-string concatenation.The message is split across two f-strings with preserved semantic meaning and correct formatting.
605-606: LGTM — Mixed string and f-string concatenation applied correctly.The verbose output message uses adjacent f-string and regular string literals, properly formatted for readability.
617-618: LGTM — CBC solver guidance message formatted with f-string chaining.The installation guidance message is correctly split across two f-strings to comply with line-length limits.
| # Flows and StorageLevel are normalised to the number of days in the non-sequential season, | ||
| # so must | ||
| # be adjusted to the number of days in the sequential season |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor wording polish in wrapped comments
These wrapped comments are correct but the standalone # so must line reads a bit awkward. If it fits under 100 chars, consider:
- # Flows and StorageLevel are normalised to the number of days in the non-sequential season,
- # so must
- # be adjusted to the number of days in the sequential season
+ # Flows and StorageLevel are normalised to the number of days in the non-sequential season,
+ # so must be adjusted to the number of days in the sequential seasonSimilarly, you could apply the same pattern where you describe normalization/adjustment for sequential seasons for slightly smoother prose.
Also applies to: 339-341, 346-348
🤖 Prompt for AI Agents
In temoa/components/storage.py around lines 191-193 (and similarly 339-341,
346-348), the wrapped comments include an awkward standalone line "# so must";
replace the fragmented line breaks with a single clear comment line under 100
characters that reads the full thought (for example: "# Flows and StorageLevel
are normalised to the number of days in the non-sequential season, so must be
adjusted to the number of days in the sequential season"), and apply the same
consolidation and wording polish to the other noted comment blocks.
| cur.execute( | ||
| "SELECT DISTINCT period FROM output_flow_out WHERE scenario is '" + str(y) + "'" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Parameterize SQL query to avoid string-based concatenation
The query for periods is built via string concatenation:
cur.execute(
"SELECT DISTINCT period FROM output_flow_out WHERE scenario is '" + str(y) + "'"
)Ruff flags this as a potential SQL injection vector (S608), and it can also break if y contains quotes. Even though y currently comes from the DB, parameterizing is safer and clearer. Suggested change:
- cur.execute(
- "SELECT DISTINCT period FROM output_flow_out WHERE scenario is '" + str(y) + "'"
- )
+ cur.execute(
+ 'SELECT DISTINCT period FROM output_flow_out WHERE scenario is ?',
+ (str(y),),
+ )This keeps behavior while eliminating concatenation-based SQL.
Based on static analysis hints (Ruff S608).
I'll verify the review comment by examining the code context and identifying which database driver is being used.
---
🏁 Script executed:
```shell
#!/bin/bash
# View the file and surrounding context
cat temoa/extensions/get_comm_tech.py | head -50
Length of output: 1455
🏁 Script executed:
#!/bin/bash
# Find database connection and imports in the file
rg -n "import|sqlite|mysql|cursor|execute" temoa/extensions/get_comm_tech.py -A2 -B2Length of output: 4083
🏁 Script executed:
#!/bin/bash
# Check for database initialization and cursor creation
fd -e py | xargs rg -l "sqlite|mysql|psycopg|cursor" | head -10Length of output: 319
🏁 Script executed:
#!/bin/bash
# Look for similar SQL concatenation patterns in the codebase
rg -n "execute.*\+" temoa/ --type py -C3Length of output: 799
🏁 Script executed:
#!/bin/bash
# Check requirements or setup files for database library
fd -e txt -e txt -e cfg -e toml -e ini | xargs cat 2>/dev/null | rg -i "sqlite|mysql|psycopg|sqlalchemy"Length of output: 5108
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for all SQL concatenation patterns in the specific file
rg -n "execute.*\+" temoa/extensions/get_comm_tech.py -C3
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for all cur.execute() calls in the file to see full scope
rg -n "cur\.execute\(" temoa/extensions/get_comm_tech.py -C2Length of output: 1837
Parameterize SQL query to prevent SQL injection vulnerability
The query at lines 32-34 concatenates the variable y directly into the SQL string, creating a SQL injection vector. Since sqlite3 is the database library in use, the fix should use the ? placeholder with tuple parameters:
- cur.execute(
- "SELECT DISTINCT period FROM output_flow_out WHERE scenario is '" + str(y) + "'"
- )
+ cur.execute(
+ 'SELECT DISTINCT period FROM output_flow_out WHERE scenario is ?',
+ (str(y),),
+ )This is the only instance of SQL concatenation in this file and should be corrected to follow secure parameterized query practices.
🧰 Tools
🪛 Ruff (0.14.5)
33-33: Possible SQL injection vector through string-based query construction
(S608)
🤖 Prompt for AI Agents
In temoа/extensions/get_comm_tech.py around lines 32 to 34, the SQL concatenates
the variable `y` into the query causing a SQL injection risk; change the call to
use sqlite3 parameterized queries by replacing the string concatenation with a
parameter placeholder (?), remove the embedded quotes around the value in the
SQL, and pass the value as a tuple argument to cur.execute (e.g.,
cur.execute("SELECT DISTINCT period FROM output_flow_out WHERE scenario = ?",
(y,))) so the database driver safely binds the parameter.
| '\nMay be non-recoverable. Possibly try a set of random vectors to initialize the ' | ||
| 'Hull.' |
There was a problem hiding this comment.
String concatenation may still exceed 100-character limit.
The error message on lines 111–112 uses implicit string concatenation to split the message across multiple lines. However, line 112 appears to still exceed the 100-character limit:
'May be non-recoverable. Possibly try a set of random vectors to initialize the 'Consider further shortening the message or restructuring the string concatenation to ensure each line respects the 100-character limit.
Apply this diff to bring line 112 into compliance:
logger.error(
'Attempt at hull construction from basis vectors failed.'
- '\nMay be non-recoverable. Possibly try a set of random vectors to initialize the '
+ '\nMay be non-recoverable. Try random vectors to initialize the '
'Hull.'
)🤖 Prompt for AI Agents
In temoа/extensions/modeling_to_generate_alternatives/hull.py around lines 111
to 112, the implicit string concatenation still produces a line exceeding the
100-character limit; shorten or reflow the message so every physical source line
is ≤100 chars — for example, break the sentence into smaller quoted pieces (each
<100 chars) concatenated across lines or reword the sentence to be more concise
(e.g., "May be non-recoverable. Try initializing the Hull with random vectors.")
so line 112 complies with the line-length rule.
| if len(tokens) != 6: | ||
| raise ValueError( | ||
| f'Incorrect number of tokens for row {row_number}. Did you omit notes / trailing comma for no notes or have a comma in your note?' | ||
| f'Error parsing line {row_number}. Did you omit notes / trailing comma for no ' | ||
| 'notes or have a comma in your note?' | ||
| ) | ||
| # convert the run number |
There was a problem hiding this comment.
Fix literal {idx} in numeric-value error and keep improved messages
The reflowed error messages in row_parser and element_locator look good and are clearer, but there’s an existing bug in the numeric conversion error:
try:
tokens[-2] = float(tokens[-2])
except ValueError:
raise ValueError('value at row {idx} must be numeric')This will literally print {idx} instead of the row number, which makes debugging CSV issues harder. Suggest:
- except ValueError:
- raise ValueError('value at row {idx} must be numeric')
+ except ValueError:
+ raise ValueError(f'value at row {row_number} must be numeric')The other rewrapped messages (row-length, missing param_name, empty indices, and index-length mismatch) are fine as-is.
Also applies to: 136-149, 292-294
🧰 Tools
🪛 Ruff (0.14.5)
119-122: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In temoa/extensions/monte_carlo/mc_run.py around lines 118-123 (and similarly at
136-149 and 292-294), the numeric conversion ValueError uses a literal '{idx}'
in the message so the row number isn't interpolated; change the exception
message to include the actual row index (e.g., use an f-string or format with
idx) so it reads the real row number, and keep the improved reflowed messages
unchanged.
| f'NOTE: Mapped target {mapped_new_table_name} not found for {old_table_name}; ' | ||
| f'using candidate {chosen_table}' | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Message reflow and INSERT construction are fine; consider validating identifiers
The NOTE/SKIP/DEBUG message splits and the multi-line insert_query preserve behavior and readability; the SQL text is unchanged apart from whitespace.
The insert_query still interpolates mapped_new_table_name and column names directly into the SQL string. Because these come from schema/PRAGMA introspection and your custom mapping, the injection risk is low, but if there is any path for untrusted names into the schema, consider adding a simple identifier validator (e.g., restricting to [A-Za-z0-9_]+) before using them here.
Also applies to: 205-207, 237-239, 257-261
1 participant
closes #214
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.