Split expected emission factors into direct and lifecycle #5420
Conversation
There was a problem hiding this comment.
The tests work as expected but we need to take a look at the types in the test itself, there are a lot of type errors in here.
I also noted that we still rely on the .ts file to get the possible modes. We should probably update this whole test to use the constants from the python parts instead so we decuple this from web.
The python constants are located at electricitymap/contrib/config/constants
tests/test_co2eq_parameters.py
Outdated
| The callback is called with the mode and factors for both the defaults | ||
| and the zone-specific overrides; each `factors` object is either a list | ||
| of factor dicts or a single factor dict, and each factor dict has the | ||
| key "value", and possibly other keys such as "source". | ||
| """ | ||
| emission_factors = cls.parameters["emissionFactors"] |
There was a problem hiding this comment.
Pylance is reporting that parameters is unknown on the type Type[CO2eqParametersDirectAndLifecycleMixin] so we have to take care of that if we want to add any form of type checking in the future.
There was a problem hiding this comment.
Note that many of the warnings were missing assert methods since the mixin class was not a subclass of TestCase.
One of the simplest ways to deal with this was to make it a subclass of TestCase; after doing that however it is found automatically by the test runner so to prevent unexpected behavior, in the latest revision I nested it inside of another class, like in https://stackoverflow.com/questions/1323455/python-unit-test-with-base-and-sub-class
tests/test_co2eq_parameters.py
Outdated
| @@ -384,6 +383,28 @@ class CO2eqParametersDirect(CO2eqParametersDirectAndLifecycleMixin, unittest.Tes | |||
|
|
|||
| parameters = CO2EQ_PARAMETERS_DIRECT | |||
|
|
|||
| # Expected min and max values for emission factors, by mode. | |||
| ranges_by_mode = { | |||
There was a problem hiding this comment.
| ranges_by_mode = { | |
| ranges_by_mode: Dict[str, Tuple[Union[int, float], Union[int, float]]] = { |
You might have to update the imports from typing after adding this.
There was a problem hiding this comment.
Done; note that I discovered that type float sort of includes int; and there's also type numbers.Number which is already imported that includes float, int and other number types.
https://stackoverflow.com/questions/50928592/mypy-type-hint-unionfloat-int-is-there-a-number-type
tests/test_co2eq_parameters.py
Outdated
| @@ -392,6 +413,28 @@ class CO2eqParametersLifecycle( | |||
|
|
|||
| parameters = CO2EQ_PARAMETERS_LIFECYCLE | |||
|
|
|||
| # Expected min and max values for emission factors, by mode. | |||
| ranges_by_mode = { | |||
There was a problem hiding this comment.
| ranges_by_mode = { | |
| ranges_by_mode: Dict[str, Tuple[Union[int, float], Union[int, float]]] = { |
|
This doesn't yet resolve 100% of the issues flagged by pyright/pylance -- I propose resolving more of these issues in a separate PR. |
For some reason numbers.Numbers does not play well with literal tuples so I changed it to a int and float union which work.
Issue
Related to #3990
Description
Splits the expected range values into direct and lifecycle.
Note that in the case of test failure, the output does have information whether the lifecycle or direct test case failed, although it may not be obvious from the message.
Double check
poetry run test_parser "zone_key"pnpx prettier --write .andpoetry run formatto format my changes.