Optimize imports for type checking with TC rules #217
Conversation
WalkthroughMove type-only imports into TYPE_CHECKING blocks and convert runtime typing.cast targets to forward-referenced string literals across the codebase; add a TC lint rule in pyproject.toml and introduce a few module-level type aliases. No runtime logic or public signatures were changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (5 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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (33)
pyproject.toml(1 hunks)temoa/_internal/exchange_tech_cost_ledger.py(3 hunks)temoa/_internal/table_data_puller.py(5 hunks)temoa/_internal/table_writer.py(2 hunks)temoa/_internal/temoa_sequencer.py(1 hunks)temoa/components/capacity.py(1 hunks)temoa/components/commodities.py(3 hunks)temoa/components/costs.py(1 hunks)temoa/components/emissions.py(1 hunks)temoa/components/flows.py(1 hunks)temoa/components/geography.py(5 hunks)temoa/components/limits.py(2 hunks)temoa/components/operations.py(1 hunks)temoa/components/reserves.py(1 hunks)temoa/components/storage.py(1 hunks)temoa/components/technology.py(3 hunks)temoa/components/utils.py(1 hunks)temoa/core/model.py(1 hunks)temoa/data_io/hybrid_loader.py(8 hunks)temoa/data_processing/make_output_plots.py(2 hunks)temoa/model_checking/commodity_graph.py(3 hunks)temoa/model_checking/network_model_data.py(3 hunks)temoa/model_checking/validators.py(1 hunks)temoa/utilities/graph_utils.py(2 hunks)temoa/utilities/set_spitter.py(1 hunks)temoa/utilities/visualizer.py(1 hunks)tests/test_emission_results.py(1 hunks)tests/test_exchange_cost_ledger.py(1 hunks)tests/test_full_runs.py(1 hunks)tests/test_network_model_data.py(5 hunks)tests/test_pricing_check.py(4 hunks)tests/test_source_check.py(2 hunks)tests/test_validators.py(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 165
File: temoa/components/time.py:229-236
Timestamp: 2025-10-18T15:54:24.846Z
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.
📚 Learning: 2025-10-27T15:53:41.829Z
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 177
File: temoa/model_checking/commodity_network.py:26-33
Timestamp: 2025-10-27T15:53:41.829Z
Learning: The Temoa project requires Python 3.12 or above as the minimum supported version, so PEP 695 `type` syntax for type aliases is appropriate and preferred over `TypeAlias`.
Applied to files:
tests/test_pricing_check.pytemoa/model_checking/validators.pytemoa/_internal/table_data_puller.py
📚 Learning: 2025-11-03T22:55:02.519Z
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 186
File: temoa/_internal/temoa_sequencer.py:60-63
Timestamp: 2025-11-03T22:55:02.519Z
Learning: In `TemoaSequencer.__init__`, when a `mode_override` is provided, it must be written back to `self.config.scenario_mode` (not just stored in `self.temoa_mode`) because `HybridLoader.create_data_dict()` has strict consistency checks that validate `config.scenario_mode` matches the presence/absence of `myopic_index`. Without this synchronization, MYOPIC mode would fail with a RuntimeError when the loader is instantiated.
Applied to files:
tests/test_full_runs.py
🧬 Code graph analysis (15)
tests/test_network_model_data.py (2)
temoa/model_checking/commodity_network.py (1)
CommodityNetwork(38-335)temoa/model_checking/network_model_data.py (1)
clone(112-114)
temoa/_internal/table_writer.py (2)
temoa/_internal/data_brick.py (1)
DataBrick(22-81)temoa/core/config.py (1)
TemoaConfig(26-272)
temoa/utilities/set_spitter.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(92-1119)
temoa/utilities/visualizer.py (1)
temoa/utilities/graph_utils.py (1)
convert_graph_to_json(43-107)
tests/test_pricing_check.py (2)
temoa/model_checking/pricing_check.py (1)
check_tech_uncap(256-337)temoa/core/model.py (1)
TemoaModel(92-1119)
temoa/model_checking/commodity_graph.py (4)
temoa/utilities/graph_utils.py (2)
calculate_initial_positions(146-208)calculate_tech_graph_positions(211-259)temoa/utilities/visualizer.py (2)
make_nx_graph(48-186)nx_to_vis(189-266)temoa/core/config.py (1)
TemoaConfig(26-272)temoa/model_checking/network_model_data.py (2)
EdgeTuple(31-38)NetworkModelData(80-136)
temoa/data_io/hybrid_loader.py (4)
temoa/types/model_types.py (1)
TemoaModel(148-224)temoa/model_checking/element_checker.py (1)
ViableSet(18-157)temoa/core/config.py (1)
TemoaConfig(26-272)temoa/data_io/loader_manifest.py (1)
LoadItem(26-59)
temoa/components/commodities.py (1)
temoa/components/utils.py (1)
get_variable_efficiency(53-82)
tests/test_exchange_cost_ledger.py (2)
temoa/_internal/exchange_tech_cost_ledger.py (2)
CostType(26-34)ExchangeTechCostLedger(37-157)tests/utilities/namespace_mock.py (1)
Namespace(1-3)
temoa/_internal/exchange_tech_cost_ledger.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(92-1119)
temoa/model_checking/network_model_data.py (1)
temoa/extensions/myopic/myopic_index.py (1)
MyopicIndex(37-57)
temoa/utilities/graph_utils.py (1)
temoa/model_checking/network_model_data.py (1)
EdgeTuple(31-38)
temoa/components/limits.py (1)
temoa/components/technology.py (1)
gather_group_techs(33-39)
temoa/_internal/table_data_puller.py (5)
temoa/_internal/exchange_tech_cost_ledger.py (2)
CostType(26-34)ExchangeTechCostLedger(37-157)temoa/components/utils.py (1)
get_variable_efficiency(53-82)temoa/types/model_types.py (5)
EI(232-239)FI(242-252)SLI(255-263)CapData(266-271)FlowType(275-282)temoa/core/model.py (1)
TemoaModel(92-1119)temoa/components/costs.py (1)
fixed_or_variable_cost(275-308)
tests/test_validators.py (2)
temoa/model_checking/validators.py (4)
linked_region_check(124-136)no_slash_or_pipe(91-104)region_check(107-121)region_group_check(139-156)temoa/core/model.py (1)
TemoaModel(92-1119)
🪛 Ruff (0.14.5)
temoa/_internal/exchange_tech_cost_ledger.py
80-80: Consider f'{exporter}-{importer}' instead of string join
Replace with f'{exporter}-{importer}'
(FLY002)
81-81: Consider f'{importer}-{exporter}' instead of string join
Replace with f'{importer}-{exporter}'
(FLY002)
🔇 Additional comments (49)
temoa/data_processing/make_output_plots.py (3)
1-2: LGTM! Enables postponed annotation evaluation.Adding
from __future__ import annotationscorrectly enables PEP 563, which allows all annotations to be evaluated as strings at runtime, reducing import overhead and enabling forward references throughout the file.
9-9: LGTM! Correctly moves type-only imports into TYPE_CHECKING.The
FigureandLegendtypes are only used in annotations (lines 263, 285, 298, 312) and never accessed at runtime. Moving them into theTYPE_CHECKINGblock reduces runtime import overhead while preserving type-checking capabilities.Also applies to: 16-18
248-248: LGTM! Correct use of forward-referenced string literal in cast.Changing to
cast('Color', rgba_value)is necessary becausecast()evaluates its first argument at runtime. Withfrom __future__ import annotationsenabled, type aliases likeColormust be referenced as string literals in runtime contexts.pyproject.toml (1)
98-99: LGTM!The addition of the TC (flake8-type-checking) rule aligns perfectly with the PR objectives to optimize imports for type checking. The trailing comma on G004 is a formatting consistency improvement.
temoa/utilities/visualizer.py (1)
21-32: LGTM!The import optimization correctly moves
IterableandSequenceinto the TYPE_CHECKING block. These types are only used in function signatures (lines 49, 196) and are not accessed at runtime, making this refactor safe and aligned with the PR objectives.temoa/components/costs.py (1)
23-25: LGTM!The imports are correctly moved into the TYPE_CHECKING block. These types are used exclusively in type annotations throughout the file and are not accessed at runtime, reducing import overhead while preserving type-checking capabilities.
temoa/_internal/temoa_sequencer.py (1)
37-40: LGTM!The
pyomo.optimport is correctly moved into the TYPE_CHECKING block. It's only used for type annotations (e.g., line 69:pyomo.opt.SolverResults) and not accessed at runtime, achieving the PR's goal of reducing runtime import overhead.temoa/_internal/exchange_tech_cost_ledger.py (1)
18-21: LGTM!The imports are correctly moved into the TYPE_CHECKING block, and the code properly uses string-based forward references in the
cast()calls (lines 58, 78, 80-81) to avoid runtime dependencies on these types.temoa/components/utils.py (1)
19-20: LGTM!The
ExprLikeandExpressionimports are correctly moved into the TYPE_CHECKING block. These types are used exclusively in function signatures and return types, not accessed at runtime.temoa/components/capacity.py (1)
23-23: LGTM!The
ExprLikeimport is correctly moved into the TYPE_CHECKING block. It's used exclusively in function return type annotations throughout the file and not accessed at runtime.temoa/components/storage.py (1)
22-35: TYPE_CHECKING-only imports here are safe and match the PR goal
ExprLike,Period,Region,Season,Technology,TimeOfDay, andVintageare only used in annotations in this module andfrom __future__ import annotationsis enabled, so moving them underTYPE_CHECKINGcorrectly reduces runtime imports without changing behavior.temoa/components/reserves.py (1)
21-25: Import move under TYPE_CHECKING is appropriate here
Period,Region,Season, andTimeOfDayare only referenced in type hints, and annotations are postponed, so placing these imports inside theTYPE_CHECKINGblock cleanly satisfies the TC rules without affecting runtime behavior.temoa/utilities/set_spitter.py (1)
5-13: RestoringTemoaModelas a real import is correct hereThis module does not use postponed annotations, and the function signatures reference
TemoaModeldirectly, so importingTemoaModelat runtime is necessary to avoidNameErrorat definition time. Moving it out ofTYPE_CHECKINGcorrectly addresses TC004 without affecting behavior.temoa/components/flows.py (1)
18-33: Flow type aliases correctly scoped under TYPE_CHECKING
Commodity,Region,Period, etc., plusActiveFlowAnnualSetandActiveFlexAnnualSet, are only referenced in annotations (e.g., return types offlow_variable_annual_indices/flex_variable_annual_indices), and this module uses postponed annotations. Keeping these imports inside theTYPE_CHECKINGblock reduces runtime dependency ontemoa.typeswithout changing behavior.Also applies to: 51-68
temoa/_internal/table_writer.py (1)
32-41: TYPE_CHECKING imports and string-basedcasttargets are safe here
DataBrick,ChangeRecord,SolverResults,Period,Region,Technology,Vintage,TemoaModel,TemoaConfig,Path, andIterableare only used in annotations in this file, and annotations are postponed, so importing them only underTYPE_CHECKINGis correct and trims runtime dependencies.- Switching to
cast('int', ei.p)/cast('int', ei.v)preserves runtime behavior (sincetyping.castis a no-op) while keeping type information for static checkers.Also applies to: 256-278
temoa/components/commodities.py (1)
22-27: TYPE_CHECKING imports and stringRegioncasts keep behavior unchanged
Region,Commodity,ExprLike,Period,Season,Technology,TimeOfDay, andVintageare now imported only underTYPE_CHECKING, which is appropriate since this module uses them purely in annotations and viacast(...)for typing;from __future__ import annotationsprevents any runtime dependency.- Rewriting
cast(Region, ...)tocast('Region', ...)(including in the export/import handling for commodity balance) only affects static typing; the actual keys passed intov_flow_out,v_flow_out_annual,get_variable_efficiency, andefficiencyare still the same region strings, so the numerical behavior of the constraints is preserved.Also applies to: 445-457, 592-604, 611-619
temoa/core/model.py (1)
49-53: <!-- [scratchpad_start]
[task verify_review_comment 2/2]
[observations]
- File examined: temoa/core/model.py
- Structure: imports, module-level code, class TemoaModel with init method
- All references to
t.*andTechnologyare INSIDE the init method body:
self.process_inputs: t.ProcessInputsDict = {}self.process_outputs: t.ProcessOutputsDict = {}- ... ~50+ similar annotations inside init
self.used_techs: set[Technology] = set()- also inside init- No class-level annotations found (e.g., class Foo: attr: Type = value outside methods)
- No module-level annotations found
[analysis]- Web search confirmed: local (function-body) variable annotations are NOT evaluated at runtime per PEP 526 & 563
- All
t.*andTechnologyreferences in this file are inside init method body- Therefore, imports can safely be under TYPE_CHECKING without causing NameError
- The original review's critical premise is INCORRECT
- The review incorrectly conflates function-body annotations with class/module-level annotations
[conclusion]- Original review comment should be marked
- The code as presented (with TYPE_CHECKING imports) is valid and will NOT raise NameError
- No changes are needed to the code
[done]
</scratchpad_end] -->Original review comment is incorrect; no changes needed
Local variable annotations inside function bodies are not evaluated at runtime per PEP 526 and PEP 563. All uses of
tandTechnologyin this file occur within the__init__method body (e.g.,self.process_inputs: t.ProcessInputsDict = {}), making them function-local annotations. These annotations are not evaluated at runtime, so omittingtandTechnologyfrom the regular import scope does not causeNameError. TheTYPE_CHECKINGguard is appropriate and does not break the code.Likely an incorrect or invalid review comment.
temoa/components/limits.py (2)
26-28: LGTM! TYPE_CHECKING block properly isolates type-only imports.The type imports are correctly moved under the TYPE_CHECKING guard, reducing runtime overhead while preserving type-checking capabilities.
423-423: LGTM! Forward-reference casts eliminate runtime type dependency.The conversion from
cast(Vintage, p)tocast('Vintage', p)correctly uses forward-reference strings, allowing theVintagetype to remain in the TYPE_CHECKING block without affecting runtime behavior.Also applies to: 431-431
temoa/components/technology.py (2)
21-23: LGTM! Typing imports properly isolated.Moving
Iterablefromcollections.abcand forward-type imports into the TYPE_CHECKING block correctly eliminates their runtime overhead, as they're only used in type annotations.
37-37: LGTM! Forward-reference casts applied consistently.Both cast operations correctly use string literals for type names, maintaining type-checking support while avoiding runtime type imports.
Also applies to: 276-276
temoa/_internal/table_data_puller.py (4)
8-9: LGTM! Proper setup for deferred type evaluation.The addition of
from __future__ import annotationscombined with the TYPE_CHECKING block correctly enables forward references and isolates type-only imports from runtime.Also applies to: 24-26
191-191: LGTM! Synthetic commodity markers use forward-reference casts.The casts to
'Commodity'for string literals'construction_input'and'end_of_life_output'correctly signal to type checkers that these synthetic markers should be treated as Commodity types.Also applies to: 207-207
356-356: LGTM! Multiple forward-reference casts preserve type safety.The tuple key construction with four forward-reference casts correctly maintains type information for the
entriesdictionary key while avoiding runtime type imports.
667-688: LGTM! Emission cost calculations use consistent forward references.All cast operations in the embodied emissions section correctly use string literals for type names, maintaining consistency with the rest of the file.
temoa/utilities/graph_utils.py (2)
26-29: LGTM! Type imports correctly isolated.The TYPE_CHECKING guard properly separates type-only imports (Commodity, Sector, Technology, EdgeTuple, Iterable, Sequence) from runtime imports.
162-162: LGTM! Forward-reference cast in set comprehension.The cast to
'Commodity'correctly maintains type information while avoiding runtime import dependency.temoa/components/geography.py (2)
22-24: LGTM! TYPE_CHECKING block properly configured.Type imports (Region, ExprLike, Period, Technology, Vintage, Iterable) are correctly isolated, as they're only used in function type annotations.
41-41: LGTM! Forward-reference casts for region manipulation.All cast operations correctly use
'Region'string literals for region parsing, joining, and permutation operations, maintaining type safety without runtime overhead.Also applies to: 63-63, 76-77, 142-143
temoa/data_io/hybrid_loader.py (3)
1-2: LGTM! Deferred annotation evaluation configured.The combination of
from __future__ import annotationsand the TYPE_CHECKING block properly enables forward references while isolating type-only imports (TemoaConfig, LoadItem, Sequence).Also applies to: 41-44
347-347: LGTM! Complex generic type as forward reference.The cast using
'Sequence[tuple[ValidationPrimitive, ...]]'as a string literal correctly handles the complex generic type annotation, compatible with Python 3.12+.
549-549: LGTM! Built-in type forward references in custom loaders.All cast operations in the custom loader methods correctly use string literals (
'list[int]','int','float') for forward-reference types, maintaining consistency with the PR's typing optimization approach.Also applies to: 654-654, 681-681, 695-695, 710-710
temoa/model_checking/validators.py (1)
17-18: LGTM! Type-only imports isolated.The TYPE_CHECKING block correctly isolates type imports (core_types and pyomo Set) that are only used in function signatures throughout the validators module.
tests/test_source_check.py (1)
209-210: LGTM! Test fixtures use forward-reference casts.The test setup correctly adopts forward-reference string literals in cast operations (
'Region','Period','Vintage'), maintaining consistency with the project-wide typing optimization while preserving test functionality.Also applies to: 221-221
tests/test_network_model_data.py (2)
2-2: LGTM! TYPE_CHECKING guard correctly isolates type imports.The TYPE_CHECKING block successfully defers Period and Region imports to type-checking time only, reducing runtime overhead as intended by the TC rules.
Also applies to: 10-11
210-210: LGTM! Forward-referenced casts avoid runtime type imports.String literals in cast expressions (e.g.,
cast('Region', 'R1')) prevent runtime evaluation of the type objects while preserving static type-checking capabilities.Also applies to: 216-216, 246-246, 328-328, 412-412
tests/test_emission_results.py (1)
38-38: String-based cast is consistent with PR pattern.While
TechTestParamsis defined locally (line 24) and available at runtime, usingcast('TechTestParams', ...)maintains consistency with the project-wide pattern of forward-referenced casts. This maximizes the benefit of deferred type evaluation.tests/test_pricing_check.py (2)
1-1: LGTM! TYPE_CHECKING guard correctly defers TemoaModel import.Also applies to: 8-9
40-40: LGTM! Forward-referenced casts maintain type safety without runtime imports.The string-literal casts allow type checkers to understand the mock_model type while avoiding runtime import of TemoaModel.
Also applies to: 58-58, 72-72
tests/test_validators.py (2)
5-5: LGTM! TYPE_CHECKING block properly isolates type imports.Also applies to: 17-19
34-34: LGTM! String-based casts maintain type hints without runtime overhead.The forward-referenced casts across all test functions successfully eliminate runtime type imports while preserving static type-checking.
Also applies to: 36-36, 58-58, 60-60, 80-80, 84-84, 100-100
temoa/model_checking/commodity_graph.py (2)
5-5: LGTM! TYPE_CHECKING guard successfully defers multiple type imports.The block properly isolates Technology, TemoaConfig, NetworkModelData, core types, and collections.abc imports to type-checking time only.
Also applies to: 15-21
53-53: LGTM! Forward-referenced casts avoid runtime type evaluation.Both casts use string literals appropriately:
- Line 53:
cast('Technology', tech_name)for type annotation- Line 296:
cast('dict[str, str]', sector_colors)converts dict[Sector, str] to the expected dict[str, str] signature for nx_to_visAlso applies to: 296-296
temoa/components/operations.py (1)
22-23: LGTM! Type imports properly isolated to TYPE_CHECKING block.The ExprLike and core type imports (Period, Region, Season, Technology, TimeOfDay, Vintage) are correctly deferred to type-checking time. Combined with
from __future__ import annotations(line 13), function signatures remain properly typed without runtime imports.temoa/components/emissions.py (1)
21-22: LGTM! Type imports correctly moved to TYPE_CHECKING block.ExprLike and core type imports are properly deferred. With
from __future__ import annotations(line 12), all type annotations are stringified automatically, eliminating runtime dependencies on these imports.tests/test_exchange_cost_ledger.py (2)
1-1: LGTM! TYPE_CHECKING guard properly isolates core type imports.Also applies to: 8-9
12-18: LGTM! Module-level constants use forward-referenced casts appropriately.The string-based casts for test constants (TEST_REGION_A, TEST_REGION_B, etc.) provide type hints for static analysis while avoiding runtime type imports. At runtime, these remain simple string/int values as intended.
temoa/model_checking/network_model_data.py (2)
15-15: LGTM! Excellent optimization of type-only imports.The TYPE_CHECKING block correctly isolates all type-only imports (Region, Technology, Commodity, Sector, Period, Vintage, MyopicIndex, Callable) from runtime imports. This reduces startup overhead while preserving full type-checking capabilities. The organization aligns perfectly with the PR objectives.
Also applies to: 23-28
365-368: LGTM! String literal casts correctly maintain type safety.All
cast()calls have been systematically updated to use string literals (e.g.,cast('Region', value)) instead of direct type references. This is the correct approach since the types are now in the TYPE_CHECKING block and unavailable at runtime. The type checker will correctly interpret these forward references, and runtime behavior is unchanged sincecast()is a no-op that simply returns the value.Also applies to: 436-437, 441-441, 444-445, 451-452, 456-456, 458-459, 462-462, 465-466
| rr1 = cast('Region', '-'.join([exporter, importer])) | ||
| rr2 = cast('Region', '-'.join([importer, exporter])) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: Consider f-strings for readability.
The string joins could be replaced with f-strings for better readability:
- rr1 = cast('Region', '-'.join([exporter, importer]))
- rr2 = cast('Region', '-'.join([importer, exporter]))
+ rr1 = cast('Region', f'{exporter}-{importer}')
+ rr2 = cast('Region', f'{importer}-{exporter}')📝 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.
| rr1 = cast('Region', '-'.join([exporter, importer])) | |
| rr2 = cast('Region', '-'.join([importer, exporter])) | |
| rr1 = cast('Region', f'{exporter}-{importer}') | |
| rr2 = cast('Region', f'{importer}-{exporter}') |
🧰 Tools
🪛 Ruff (0.14.5)
80-80: Consider f'{exporter}-{importer}' instead of string join
Replace with f'{exporter}-{importer}'
(FLY002)
81-81: Consider f'{importer}-{exporter}' instead of string join
Replace with f'{importer}-{exporter}'
(FLY002)
🤖 Prompt for AI Agents
In temoa/_internal/exchange_tech_cost_ledger.py around lines 80 to 81, the
Region string construction uses '-'.join([...]) which is less readable; replace
the two assignments with f-strings that interpolate exporter and importer (e.g.,
rr1 = cast('Region', f"{exporter}-{importer}") and rr2 = cast('Region',
f"{importer}-{exporter}")) to improve clarity while preserving types and values.
tests/test_full_runs.py
Outdated
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| import pyomo.environ as pyo |
There was a problem hiding this comment.
Critical: Runtime import needed for pyo usage.
The import of pyomo.environ as pyo is guarded by TYPE_CHECKING, but Line 61 uses pyo.Param at runtime in the type annotation. This will cause a NameError when the test executes.
Apply this diff to use a string-based forward reference instead:
- efficiency_param: pyo.Param = mdl.efficiency
+ efficiency_param: 'pyo.Param' = mdl.efficiencyAlternatively, if pyo is needed at runtime elsewhere, move the import outside the TYPE_CHECKING block.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tests/test_full_runs.py around lines 16 to 19, the import of pyomo.environ as
pyo is wrapped in TYPE_CHECKING but pyo is referenced at runtime (e.g.,
pyo.Param) causing a NameError; fix by either moving "import pyomo.environ as
pyo" out of the TYPE_CHECKING block so pyo is available at runtime, or change
the runtime type annotation to a string forward reference (e.g., "pyo.Param")
wherever pyo types are used at runtime; update all affected annotations
accordingly so tests run without NameError.
There was a problem hiding this comment.
clearly the test is running in CI and passing right now, so this analysis can't be correct
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Python local variable type annotations runtime evaluation TYPE_CHECKING
💡 Result:
Short summary and guidance:
-
By default (pre-PEP 563 behavior), function parameter and return annotations are evaluated at function definition time; local-variable annotations are not evaluated (they are recorded in the function’s annotations / are local-only) so forward references in local annotations usually don’t need to be quoted. [1][2]
-
PEP 563 ("postponed evaluation of annotations", opt-in via from future import annotations, and the stringified-annotations behavior) makes annotations be stored as strings; that affects runtime resolution and means you must explicitly evaluate them (e.g., typing.get_type_hints(obj, globalns, localns) or eval(...)). PEP 563 notes local-state access from annotations is not supported and local annotations are treated differently. [1]
-
typing.TYPE_CHECKING is a constant that is True for static type-checkers and False at runtime. Use:
if TYPE_CHECKING:
import expensive_mod
to avoid runtime imports while still letting type checkers see the name. But if you later try to resolve annotations at runtime (get_type_hints), names imported only under TYPE_CHECKING will be missing unless you provide appropriate globalns/localns or import them at runtime as well. [1][2]
Practical recipes:
-
If you want to avoid runtime evaluation and forward-ref quoting pain, add at top:
from future import annotations
then use typing.get_type_hints(...) when you need resolved types at runtime. [1][2] -
If you keep normal annotations (no future import) and want to avoid heavy runtime imports, put imports inside if TYPE_CHECKING:, and use string forward-references for parameter/return annotations that reference those names. If you need to resolve later, import the names at runtime or pass them into get_type_hints via globalns/localns. [2]
References:
[1] PEP 563 — Postponed Evaluation of Annotations.
[2] Python typing docs — typing.TYPE_CHECKING and get_type_hints.
[3] Community discussion / examples (Stack Overflow) on TYPE_CHECKING, forward refs, and from future import annotations.
@ParticularlyPythonicBS You're absolutely right, and I apologize for the incorrect analysis.
The key distinction I missed is that local variable annotations inside functions are not evaluated at runtime (under Python's default pre-PEP 563 behavior). They're simply stored as metadata but never executed. Only function parameter and return annotations are evaluated at function definition time.
Since line 61 uses pyo.Param as a local variable annotation:
efficiency_param: pyo.Param = mdl.efficiency...and not as a parameter or return type, the pyo.Param expression is never evaluated at runtime. That's why the test passes successfully even though pyo is only imported under TYPE_CHECKING.
The code is correct as-is. Thank you for catching this!
✏️ Learnings added
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 217
File: tests/test_full_runs.py:16-19
Timestamp: 2025-11-21T03:43:36.876Z
Learning: In Python (without `from __future__ import annotations`), local variable type annotations inside functions are NOT evaluated at runtime - they are only stored as metadata. Only function parameter annotations and return type annotations are evaluated at function definition time. Therefore, types imported under `TYPE_CHECKING` can safely be used in local variable annotations without causing NameError, but would cause issues if used in parameter or return annotations.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 165
File: temoa/components/time.py:229-236
Timestamp: 2025-10-18T15:54:24.846Z
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.
…003) - Moved 239 type-only imports into TYPE_CHECKING blocks - Added TYPE_CHECKING imports where needed - Quoted type expressions in cast() calls for forward references - Optimizes runtime import overhead
- Added 'from __future__ import annotations' to 12 files - Removed manual quotes from cast() type expressions - Enables PEP 563 postponed evaluation of annotations - Cleaner and more maintainable than manual quoting
ParticularlyPythonicBS
force-pushed
the
lint/type-checking-imports
branch
from
80b066d to
02d751e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (34)
docs/source/conf.py(2 hunks)pyproject.toml(1 hunks)temoa/_internal/exchange_tech_cost_ledger.py(3 hunks)temoa/_internal/table_data_puller.py(5 hunks)temoa/_internal/table_writer.py(2 hunks)temoa/_internal/temoa_sequencer.py(2 hunks)temoa/components/capacity.py(1 hunks)temoa/components/commodities.py(3 hunks)temoa/components/costs.py(1 hunks)temoa/components/emissions.py(1 hunks)temoa/components/flows.py(1 hunks)temoa/components/geography.py(5 hunks)temoa/components/limits.py(2 hunks)temoa/components/operations.py(1 hunks)temoa/components/reserves.py(1 hunks)temoa/components/storage.py(1 hunks)temoa/components/technology.py(3 hunks)temoa/components/utils.py(1 hunks)temoa/core/model.py(2 hunks)temoa/data_io/hybrid_loader.py(7 hunks)temoa/data_processing/make_output_plots.py(2 hunks)temoa/model_checking/commodity_graph.py(3 hunks)temoa/model_checking/network_model_data.py(3 hunks)temoa/model_checking/validators.py(1 hunks)temoa/utilities/graph_utils.py(2 hunks)temoa/utilities/set_spitter.py(1 hunks)temoa/utilities/visualizer.py(1 hunks)tests/test_emission_results.py(1 hunks)tests/test_exchange_cost_ledger.py(1 hunks)tests/test_full_runs.py(2 hunks)tests/test_network_model_data.py(5 hunks)tests/test_pricing_check.py(4 hunks)tests/test_source_check.py(2 hunks)tests/test_validators.py(5 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 217
File: tests/test_full_runs.py:16-19
Timestamp: 2025-11-21T03:43:36.876Z
Learning: In Python (without `from __future__ import annotations`), local variable type annotations inside functions are NOT evaluated at runtime - they are only stored as metadata. Only function parameter annotations and return type annotations are evaluated at function definition time. Therefore, types imported under `TYPE_CHECKING` can safely be used in local variable annotations without causing NameError, but would cause issues if used in parameter or return annotations.
📚 Learning: 2025-11-21T03:43:36.876Z
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 217
File: tests/test_full_runs.py:16-19
Timestamp: 2025-11-21T03:43:36.876Z
Learning: In Python (without `from __future__ import annotations`), local variable type annotations inside functions are NOT evaluated at runtime - they are only stored as metadata. Only function parameter annotations and return type annotations are evaluated at function definition time. Therefore, types imported under `TYPE_CHECKING` can safely be used in local variable annotations without causing NameError, but would cause issues if used in parameter or return annotations.
Applied to files:
tests/test_exchange_cost_ledger.pytemoa/components/storage.pytemoa/components/costs.pytemoa/_internal/table_data_puller.pytemoa/core/model.pytemoa/components/operations.pytests/test_full_runs.pytemoa/components/utils.py
📚 Learning: 2025-10-27T15:53:41.829Z
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 177
File: temoa/model_checking/commodity_network.py:26-33
Timestamp: 2025-10-27T15:53:41.829Z
Learning: The Temoa project requires Python 3.12 or above as the minimum supported version, so PEP 695 `type` syntax for type aliases is appropriate and preferred over `TypeAlias`.
Applied to files:
tests/test_pricing_check.pytemoa/_internal/table_data_puller.pytests/test_full_runs.pytemoa/model_checking/validators.py
📚 Learning: 2025-11-03T22:55:02.519Z
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 186
File: temoa/_internal/temoa_sequencer.py:60-63
Timestamp: 2025-11-03T22:55:02.519Z
Learning: In `TemoaSequencer.__init__`, when a `mode_override` is provided, it must be written back to `self.config.scenario_mode` (not just stored in `self.temoa_mode`) because `HybridLoader.create_data_dict()` has strict consistency checks that validate `config.scenario_mode` matches the presence/absence of `myopic_index`. Without this synchronization, MYOPIC mode would fail with a RuntimeError when the loader is instantiated.
Applied to files:
temoa/data_io/hybrid_loader.py
📚 Learning: 2025-10-18T15:54:24.846Z
Learnt from: ParticularlyPythonicBS
Repo: TemoaProject/temoa PR: 165
File: temoa/components/time.py:229-236
Timestamp: 2025-10-18T15:54:24.846Z
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:
tests/test_full_runs.py
🧬 Code graph analysis (19)
temoa/components/geography.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(93-1120)
tests/test_exchange_cost_ledger.py (2)
temoa/_internal/exchange_tech_cost_ledger.py (2)
CostType(24-32)ExchangeTechCostLedger(35-155)tests/utilities/namespace_mock.py (1)
Namespace(1-3)
tests/test_pricing_check.py (2)
temoa/model_checking/pricing_check.py (1)
check_tech_uncap(256-337)temoa/core/model.py (1)
TemoaModel(93-1120)
temoa/components/costs.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(93-1120)
tests/test_validators.py (2)
temoa/model_checking/validators.py (4)
linked_region_check(132-144)no_slash_or_pipe(99-112)region_check(115-129)region_group_check(147-164)temoa/core/model.py (1)
TemoaModel(93-1120)
temoa/components/commodities.py (1)
temoa/components/utils.py (1)
get_variable_efficiency(53-82)
temoa/_internal/exchange_tech_cost_ledger.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(93-1120)
temoa/_internal/table_data_puller.py (3)
temoa/types/model_types.py (2)
EI(232-239)FI(242-252)temoa/core/model.py (1)
TemoaModel(93-1120)temoa/components/costs.py (1)
fixed_or_variable_cost(275-308)
temoa/data_io/hybrid_loader.py (3)
temoa/model_checking/element_checker.py (1)
ViableSet(18-157)temoa/core/config.py (1)
TemoaConfig(26-272)temoa/data_io/loader_manifest.py (1)
LoadItem(26-59)
temoa/model_checking/commodity_graph.py (4)
temoa/utilities/graph_utils.py (2)
calculate_initial_positions(146-208)calculate_tech_graph_positions(211-259)temoa/utilities/visualizer.py (2)
make_nx_graph(49-187)nx_to_vis(190-267)temoa/core/config.py (1)
TemoaConfig(26-272)temoa/model_checking/network_model_data.py (2)
EdgeTuple(31-38)NetworkModelData(80-136)
temoa/utilities/visualizer.py (1)
temoa/utilities/graph_utils.py (1)
convert_graph_to_json(43-107)
temoa/model_checking/network_model_data.py (1)
temoa/extensions/myopic/myopic_index.py (1)
MyopicIndex(37-57)
temoa/_internal/table_writer.py (4)
temoa/_internal/data_brick.py (1)
DataBrick(22-81)temoa/core/config.py (1)
TemoaConfig(26-272)temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/types/model_types.py (1)
TemoaModel(148-224)
tests/test_network_model_data.py (1)
temoa/model_checking/commodity_network.py (1)
CommodityNetwork(38-335)
temoa/components/limits.py (1)
temoa/components/technology.py (1)
gather_group_techs(33-39)
temoa/components/utils.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(93-1120)
temoa/utilities/set_spitter.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(93-1120)
temoa/model_checking/validators.py (2)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(93-1120)
temoa/utilities/graph_utils.py (1)
temoa/model_checking/network_model_data.py (1)
EdgeTuple(31-38)
🪛 Ruff (0.14.5)
temoa/_internal/exchange_tech_cost_ledger.py
78-78: Consider f'{exporter}-{importer}' instead of string join
Replace with f'{exporter}-{importer}'
(FLY002)
79-79: Consider f'{importer}-{exporter}' instead of string join
Replace with f'{importer}-{exporter}'
(FLY002)
🔇 Additional comments (38)
temoa/components/utils.py (2)
18-19: LGTM! Expression moved to TYPE_CHECKING correctly.The
Expressiontype frompyomo.coreis only used in function parameter annotations (line 42). Withfrom __future__ import annotationspresent, all annotations are stored as strings and not evaluated at runtime, making this move safe and achieving the PR's goal of reducing runtime import overhead.
23-23: LGTM! ExprLike correctly placed in TYPE_CHECKING.The
ExprLiketype is only used in the return type annotation (line 42). With postponed annotations enabled, this import optimization is safe and aligns with the PR objectives.temoa/utilities/graph_utils.py (2)
24-28: LGTM! Type-only imports correctly moved to TYPE_CHECKING block.The imports are correctly placed under
TYPE_CHECKINGsince they're only used in type annotations. Withfrom __future__ import annotationson line 10, all annotations are stringified at runtime, so these imports won't cause NameErrors. This successfully reduces runtime import overhead as intended by the TC rules.
162-162: LGTM! Forward-referenced cast correctly prevents runtime NameError.The conversion from
cast(Commodity, n)tocast('Commodity', n)is necessary and correct. SinceCommodityis now only imported underTYPE_CHECKING, using the string forward reference prevents a NameError at runtime while maintaining the same type-checking behavior.pyproject.toml (1)
100-100: LGTM! TC lint rule addition aligns with PR objective.The addition of the "TC" (flake8-type-checking) rule enables automated enforcement of the type-checking import optimizations implemented throughout this PR.
temoa/components/capacity.py (1)
21-31: LGTM! Type-only imports correctly moved to TYPE_CHECKING block.With
from __future__ import annotationspresent (line 12), all type annotations are stored as strings and not evaluated at runtime. MovingExprLikeand type aliases to the TYPE_CHECKING block safely reduces runtime import overhead without risking NameError exceptions.temoa/components/costs.py (1)
21-26: LGTM! Pyomo type imports correctly deferred to TYPE_CHECKING.With
from __future__ import annotationsactive (line 14), the Pyomo types (Expression,Var,ComponentData) are only needed for static type checking. Moving them to the TYPE_CHECKING block appropriately reduces runtime import overhead while preserving type safety.temoa/components/operations.py (1)
20-23: LGTM! Type imports properly guarded with TYPE_CHECKING.The type-only imports are correctly placed behind the TYPE_CHECKING guard. With
from __future__ import annotations(line 13), these types are only needed during static analysis, not at runtime.temoa/utilities/visualizer.py (1)
21-33: LGTM! Collections.abc types correctly deferred to TYPE_CHECKING.With
from __future__ import annotationsactive (line 13),IterableandSequencefrom collections.abc are only needed for static type checking. Moving them behind the TYPE_CHECKING guard appropriately reduces runtime dependencies.temoa/components/storage.py (1)
25-26: LGTM! Type imports appropriately guarded.The type-only imports are correctly moved to the TYPE_CHECKING block. With
from __future__ import annotations(line 14), these types are evaluated as strings and don't require runtime availability.tests/test_pricing_check.py (2)
1-9: LGTM! TemoaModel import correctly guarded with TYPE_CHECKING.Since
TemoaModelis only used in forward-referenced casts (cast('TemoaModel', ...)) and not in evaluated runtime code or parameter annotations, moving it behind the TYPE_CHECKING guard is safe and reduces test runtime imports.
40-40: LGTM! Forward-referenced cast works correctly with TYPE_CHECKING.The string literal cast
cast('TemoaModel', mock_model)allows the type hint to work during static analysis without requiring theTemoaModeltype to be available at runtime.tests/test_source_check.py (1)
209-221: This pattern is intentional and consistent across the entire test suite—no action needed.The types
Region,Period,Vintage, andTechnologyare properly imported at line 18 fromtemoa/types/core_types.py, where they're defined asNewTypedeclarations. While direct references (e.g.,cast(Region, 'test_region')) would technically work, the forward-referenced string cast pattern (e.g.,cast('Region', 'test_region')) is an established convention used consistently throughout all test files, includingtest_exchange_cost_ledger.py,test_network_model_data.py, and others. This is a deliberate stylistic choice for consistency.temoa/components/emissions.py (1)
19-30: LGTM! Type-only imports correctly moved to TYPE_CHECKING block.The imports of
ExprLikeand core types are only used in type annotations (function parameters and return types). Withfrom __future__ import annotationsat line 12, these annotations are postponed and evaluated as strings, so the imports are only needed at type-checking time.temoa/components/technology.py (2)
20-24: LGTM! Iterable and custom types correctly moved to TYPE_CHECKING.The
Iterableimport fromcollections.abcis only used in the return type annotation and is correctly placed under theTYPE_CHECKINGguard. With postponed annotations, this reduces runtime import overhead.
37-37: String casts correctly align with TYPE_CHECKING imports.The casts now use forward-referenced string literals (
'Technology'and'Period') which is consistent with these types being imported underTYPE_CHECKING. This ensures the code works correctly at runtime when these types aren't actually imported.Also applies to: 276-276
temoa/core/model.py (1)
12-12: LGTM! TYPE_CHECKING guard correctly introduced for type-only imports.The imports of custom type modules (
typesandcore_types.Technology) are correctly placed underTYPE_CHECKINGsince they're only used for type annotations. Withfrom __future__ import annotations, all annotations are postponed, making these imports unnecessary at runtime.Also applies to: 51-53
temoa/utilities/set_spitter.py (1)
7-7: LGTM! Unconditional import is correct for runtime usage.Unlike other modules in this PR,
TemoaModelis correctly imported unconditionally here because it's used at runtime in the function bodies (e.g.,model.component_map(ctype=pyo.Set)at line 17). The TYPE_CHECKING guard would be incorrect since the code needs runtime access to the model's methods and attributes.temoa/_internal/table_writer.py (1)
32-43: LGTM! Custom type imports correctly moved to TYPE_CHECKING.The imports of
Path,SolverResults,DataBrick,TemoaConfig,TemoaModel,ChangeRecord, and custom core types are correctly placed underTYPE_CHECKINGsince they're only used in type annotations. With postponed annotations, this reduces runtime import overhead.temoa/model_checking/validators.py (1)
15-27: LGTM! Type-only imports correctly moved to TYPE_CHECKING.The imports of
Setfrompyomo.coreand custom core types fromtemoa.types.core_typesare correctly placed underTYPE_CHECKING. These types are only used in function parameter and return type annotations, and with postponed annotations, they're not needed at runtime.tests/test_validators.py (2)
5-5: LGTM! TYPE_CHECKING guard correctly added for test imports.The imports of
TemoaModelandRegionare correctly placed underTYPE_CHECKINGsince they're only used in type annotations (specifically in the cast operations). This reduces runtime import overhead in the test suite.Also applies to: 17-19
34-38: String casts correctly align with TYPE_CHECKING imports.The cast operations now use forward-referenced string literals (
'TemoaModel'and'Region') which is consistent with these types being imported underTYPE_CHECKING. This ensures the test code works correctly even though the types aren't actually imported at runtime.Also applies to: 58-62, 80-86, 100-100
temoa/_internal/exchange_tech_cost_ledger.py (1)
18-19: LGTM: TYPE_CHECKING imports properly implemented.The imports are correctly guarded behind TYPE_CHECKING, and the corresponding runtime casts use forward-referenced strings throughout the file (lines 56, 76, 78-79). This reduces runtime import overhead while preserving type-checking functionality.
tests/test_network_model_data.py (1)
10-11: LGTM: TYPE_CHECKING imports correctly applied in tests.The Period and Region type imports are properly deferred to type-checking time, and all cast operations throughout the test file (lines 210, 216, 246, 328, 412) correctly use forward-referenced string literals. Based on learnings.
temoa/components/commodities.py (1)
22-25: LGTM: Comprehensive TYPE_CHECKING refactor.The types Season, Technology, TimeOfDay, Vintage, and additional types are properly moved behind TYPE_CHECKING. All corresponding runtime casts throughout the file (lines 448-450, 457-459, 595-606, 614, 621) correctly use forward-referenced strings.
temoa/model_checking/commodity_graph.py (1)
15-20: LGTM: TYPE_CHECKING imports properly structured.The imports are correctly guarded, and the corresponding casts on lines 53 and 296 properly use forward-referenced string literals for Technology and dict type hints.
temoa/components/geography.py (1)
22-25: LGTM: Geography module TYPE_CHECKING refactor.The TYPE_CHECKING block properly guards Iterable and type imports, and all runtime casts (lines 41, 63, 76-77, 142-143) correctly use forward-referenced string literals for Region types.
temoa/_internal/table_data_puller.py (1)
24-26: LGTM: TYPE_CHECKING imports correctly implemented.The TYPE_CHECKING block properly guards imports, and with
from __future__ import annotationson line 9, all forward-referenced casts throughout the file are safe. Changes are consistent with the PR objectives.temoa/model_checking/network_model_data.py (1)
23-27: TYPE_CHECKING imports correctly structured.The TYPE_CHECKING block properly guards type-only imports, and most forward-referenced casts throughout the file are correct.
temoa/components/limits.py (1)
27-28: LGTM: TYPE_CHECKING imports properly applied.The type imports are correctly moved behind TYPE_CHECKING, and the corresponding casts on lines 422 and 430 properly use forward-referenced string literals for the Vintage type.
temoa/data_io/hybrid_loader.py (1)
22-29: TYPE_CHECKING imports and string-literalcasttargets look correct and safe
- Adding
from __future__ import annotationsplus importingSequence,TemoaConfig, andLoadItemonly underTYPE_CHECKINGis sound: they’re used purely in annotations, so no runtime dependency is introduced.- Converting
cast(Sequence[...], ...),cast(list[int], ...),cast(int, ...), andcast(float, ...)to use string-literal type arguments does not affect behavior (sincetyping.castis a no-op at runtime) and aligns with your TC rules.I don’t see any new runtime NameError risks or behavior changes from these typing tweaks.
Also applies to: 41-46, 348-351, 550-551, 655-656, 682-685, 696-700, 710-712
temoa/data_processing/make_output_plots.py (1)
1-2: Matplotlib TYPE_CHECKING imports and color/type aliases are consistent with runtime behaviorThe additions here look good:
FigureandLegendare only used in annotations, and withfrom __future__ import annotationsit’s correct to import them underTYPE_CHECKINGto avoid extra runtime imports.- The new aliases (
DbRow,PlotData,Color,ColorMapFunc) match how the data and RGBA tuples are actually used in plotting.get_cmap’swrapperstill returns the same RGBA object from matplotlib; wrapping it incast('Color', ...)only improves type checking and doesn’t alter runtime behavior.No issues spotted with backend selection or plotting logic.
Also applies to: 8-10, 16-19, 22-28, 238-249, 263-287, 298-315
tests/test_full_runs.py (1)
7-8: Safe use ofpyomo.environunderTYPE_CHECKINGwith local annotationGating
import pyomo.environ as pyobehindTYPE_CHECKINGis fine here becausepyois only referenced in a local variable annotation:efficiency_param: pyo.Param = mdl.efficiencyLocal annotations are not evaluated at runtime, so this does not require
pyoto exist when the test runs. You still importSolverResultsat runtime for the function parameter annotation, which avoids NameErrors there. This change cleanly reduces runtime imports without breaking the test.Also applies to: 18-20, 61-65
tests/test_emission_results.py (1)
38-45: String-literalcasttoTechTestParamsis behaviorally neutralSwitching to
param = cast('TechTestParams', request.param)keeps runtime behavior identical (sincecastis a no-op) while satisfying the TC pattern for forward-referenced types. The fixture logic and downstream uses ofparam[...]remain unchanged.temoa/components/reserves.py (1)
11-17: TYPE_CHECKING gating for core types in reserves component is correctGiven
from __future__ import annotationsat the top, usingTemoaModel,ExprLike, and the core types (Region,Period,Season,TimeOfDay) only in annotations and importing them underTYPE_CHECKINGis safe. All runtime logic continues to work solely with themodelinstance and Pyomo objects, so this change meets the TC rules without affecting reserve-margin behavior.Also applies to: 20-24, 32-33, 50-52, 165-167, 248-250
temoa/components/flows.py (1)
12-19: Flow/commodity index functions use TYPE_CHECKING-only types purely in annotationsThe added TYPE_CHECKING block for
TemoaModel, theActive*set aliases, and the core types is consistent withfrom __future__ import annotations: those names are only referenced in annotations, while all runtime logic usesmodelattributes likeactive_flow_rpsditvo,active_flex_rpitvo, etc. This cleanly satisfies the TC rules without changing how flow index sets are built.Also applies to: 20-31, 42-47, 50-53, 56-62, 64-67, 70-75, 78-83, 91-110
tests/test_exchange_cost_ledger.py (1)
1-10: Exchange ledger tests: TYPE_CHECKING imports and stringcasttargets are safeMoving
Period,Region,Technology, andVintageinto a TYPE_CHECKING block and updating the module-level constants to usecast('Region', ...),cast('Period', ...), etc. only affects typing. The tests still operate on plain strings/ints as keys, andtyping.castdoesn’t change values at runtime, so there’s no behavioral impact while satisfying the TC rules.Also applies to: 12-18
temoa/_internal/temoa_sequencer.py (1)
11-12: ****The review comment mischaracterizes the code. Line 68's
self.pf_results: pyomo.opt.SolverResults | None = Noneis an instance attribute annotation inside the__init__method, not a class-level annotation. Per Python semantics (and consistent with the learnings provided), instance attribute annotations inside functions are stored as metadata and are not evaluated at runtime—only function parameter and return type annotations are evaluated at function definition time. Therefore, the annotation will not cause aNameErroreven thoughpyomo.optis imported only underTYPE_CHECKING. The code is correct as written and requires no changes.Likely an incorrect or invalid review comment.
| project_metadata = cast('dict[str, Any]', pyproject_data['project']) | ||
| project = 'Tools for Energy Model Optimization and Analysis (Temoa)' | ||
| author = ', '.join( | ||
| author['name'] for author in cast(list[dict[str, Any]], project_metadata.get('authors', [])) | ||
| author['name'] for author in cast('list[dict[str, Any]]', project_metadata.get('authors', [])) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unnecessary string casts for built-in types.
The changes use string literals for built-in types ('dict[str, Any]', 'list[dict[str, Any]]', 'str') in casts. Built-in types are always available at runtime and don't need to be moved behind TYPE_CHECKING guards or forward-referenced as strings. This differs from the pattern applied elsewhere in the PR where custom types (e.g., Region, Technology) are forward-referenced because they're imported under TYPE_CHECKING.
Consider reverting these casts to use the types directly:
-project_metadata = cast('dict[str, Any]', pyproject_data['project'])
+project_metadata = cast(dict[str, Any], pyproject_data['project'])
project = 'Tools for Energy Model Optimization and Analysis (Temoa)'
author = ', '.join(
- author['name'] for author in cast('list[dict[str, Any]]', project_metadata.get('authors', []))
+ author['name'] for author in cast(list[dict[str, Any]], project_metadata.get('authors', []))
)
copyright = f'2011-{time.strftime("%Y")}, NC State University'
# The short X.Y version
-version = cast('str', project_metadata['version']).rsplit('.', 1)[
+version = cast(str, project_metadata['version']).rsplit('.', 1)[
0
] # '4.0.0a1.dev20251113' -> '4.0.0a1'Also applies to: 36-36
🤖 Prompt for AI Agents
In docs/source/conf.py around lines 28 to 31 (and similarly at line 36), the
cast calls use string literal forward-references for built-in typing constructs
like 'dict[str, Any]' and 'list[dict[str, Any]]'; replace these with the actual
types (dict[str, Any], list[dict[str, Any]], str) in the cast calls so they are
not string-literals — i.e., remove the quotes around the type expressions in
cast(...) and any similar occurrence at line 36, keeping the rest of the logic
unchanged.
| entry = (scenario, ei.r, sector, cast('int', ei.p), ei.e, ei.t, ei.v, val) | ||
| else: # embodied emissions | ||
| entry = (scenario, ei.r, sector, cast(int, ei.v), ei.e, ei.t, ei.v, val) | ||
| entry = (scenario, ei.r, sector, cast('int', ei.v), ei.e, ei.t, ei.v, val) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unnecessary string cast for built-in type int.
Similar to the issue in docs/source/conf.py, the casts use string literals for the built-in type int. Built-in types are always available at runtime and don't need forward references.
Consider using the type directly:
if hasattr(ei, 'p'): # emissions from flows
- entry = (scenario, ei.r, sector, cast('int', ei.p), ei.e, ei.t, ei.v, val)
+ entry = (scenario, ei.r, sector, cast(int, ei.p), ei.e, ei.t, ei.v, val)
else: # embodied emissions
- entry = (scenario, ei.r, sector, cast('int', ei.v), ei.e, ei.t, ei.v, val)
+ entry = (scenario, ei.r, sector, cast(int, ei.v), ei.e, ei.t, ei.v, val)📝 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.
| entry = (scenario, ei.r, sector, cast('int', ei.p), ei.e, ei.t, ei.v, val) | |
| else: # embodied emissions | |
| entry = (scenario, ei.r, sector, cast(int, ei.v), ei.e, ei.t, ei.v, val) | |
| entry = (scenario, ei.r, sector, cast('int', ei.v), ei.e, ei.t, ei.v, val) | |
| entry = (scenario, ei.r, sector, cast(int, ei.p), ei.e, ei.t, ei.v, val) | |
| else: # embodied emissions | |
| entry = (scenario, ei.r, sector, cast(int, ei.v), ei.e, ei.t, ei.v, val) |
🤖 Prompt for AI Agents
In temoa/_internal/table_writer.py around lines 278 to 280, the code uses
cast('int', ...) with the built-in type name as a string; replace these
string-forward-reference casts with the actual type object (e.g., cast(int,
ei.p) and cast(int, ei.v)) so the typing.cast receives the int type directly
(or, if you prefer, convert values with int(...) instead); update both
occurrences on the shown lines.
| construction_lifetime = basic_data['period_length'].get( | ||
| cast('Period', v), cast('Period', 1) |
There was a problem hiding this comment.
Fix incorrect cast in dictionary default value.
The default value for dict.get() is incorrectly cast to 'Period'. Since basic_data['period_length'] is of type dict[Period, int], the default value should be an int, not a Period. The construction_lifetime variable is later used as the lifetime parameter in EdgeTuple (line 463), which expects int | None.
Apply this diff:
- construction_lifetime = basic_data['period_length'].get(
- cast('Period', v), cast('Period', 1)
- )
+ construction_lifetime = basic_data['period_length'].get(
+ cast('Period', v), 1
+ )🤖 Prompt for AI Agents
In temoa/model_checking/network_model_data.py around lines 451 to 452, the
default value passed to basic_data['period_length'].get() is incorrectly cast as
'Period' when it should be an int; change the default from cast('Period', 1) to
an integer (e.g., cast(int, 1) or simply 1) so construction_lifetime is an int
(or None) matching the EdgeTuple.lifetime type.
closes #212
Summary by CodeRabbit
Refactor
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.