From 9e461f472fd4f91caa6aa8de5aa43da248850952 Mon Sep 17 00:00:00 2001 From: Javier Lopez Lorente Date: Tue, 13 Oct 2026 15:50:23 +0200 Subject: [PATCH 01/12] Add support for case-insensitive data types in PVSystem class --- solarfarmer/models/pvsystem/plant_defaults.py | 19 ++- tests/test_pvsystem.py | 137 +++++++++++++++++- 2 files changed, 152 insertions(+), 4 deletions(-) diff --git a/solarfarmer/models/pvsystem/plant_defaults.py b/solarfarmer/models/pvsystem/plant_defaults.py index 39b147f..db016c2 100644 --- a/solarfarmer/models/pvsystem/plant_defaults.py +++ b/solarfarmer/models/pvsystem/plant_defaults.py @@ -10,21 +10,34 @@ # ------------------------------------------------------------------------------ -class InverterType(str, Enum): +class _CaseInsensitiveStrEnum(str, Enum): + """String enum whose lookup by value ignores case (e.g. ``"fixed"`` -> ``FIXED``).""" + + @classmethod + def _missing_(cls, value): + if isinstance(value, str): + folded = value.strip().casefold() + for member in cls: + if member.value.casefold() == folded: + return member + return None + + +class InverterType(_CaseInsensitiveStrEnum): """Enumeration for inverter types.""" STRING = "String" CENTRAL = "Central" -class MountingType(str, Enum): +class MountingType(_CaseInsensitiveStrEnum): """Enumeration for mounting types.""" FIXED = "Fixed" TRACKER = "Tracker" -class OrientationType(str, Enum): +class OrientationType(_CaseInsensitiveStrEnum): """Enumeration for module orientation types.""" PORTRAIT = "Portrait" diff --git a/tests/test_pvsystem.py b/tests/test_pvsystem.py index 3d65604..a1d9d16 100644 --- a/tests/test_pvsystem.py +++ b/tests/test_pvsystem.py @@ -3,7 +3,7 @@ import pytest -from solarfarmer import PVSystem +from solarfarmer import InverterType, MountingType, OrientationType, PVSystem from solarfarmer.models.pvsystem.pvsystem import construct_plant @@ -178,6 +178,141 @@ def test_pvsystem_mounting_type_tracker(self): assert plant.mounting == "Tracker" + @pytest.mark.parametrize("value", ["fixed", "FIXED", "Fixed", " fixed "]) + def test_pvsystem_mounting_case_insensitive(self, value): + plant = PVSystem(latitude=45.0, longitude=10.0, mounting=value) + + assert plant.mounting is MountingType.FIXED + + def test_pvsystem_orientation_and_inverter_case_insensitive(self): + plant = PVSystem( + latitude=45.0, + longitude=10.0, + module_orientation="LANDSCAPE", + inverter_type="string", + ) + + assert plant.module_orientation is OrientationType.LANDSCAPE + assert plant.inverter_type is InverterType.STRING + + def test_pvsystem_enum_fields_normalized_on_reassignment(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + plant.mounting = "tracker" + plant.module_orientation = "landscape" + plant.inverter_type = "STRING" + + assert plant.mounting is MountingType.TRACKER + assert plant.module_orientation is OrientationType.LANDSCAPE + assert plant.inverter_type is InverterType.STRING + + def test_pvsystem_enum_reassignment_invalid_value_raises(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + with pytest.raises(ValueError, match="MountingType"): + plant.mounting = "Rooftop" + with pytest.raises(ValueError, match="MountingType"): + plant.mounting = None + + def test_autofilled_defaults_follow_input_changes(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + assert plant.mounting_height == 0.7 + assert plant.dc_ohmic_loss == 0.015 + + plant.mounting = "tracker" + plant.inverter_type = "string" + plant.bifacial = True + plant.transformer_stages = 0 + + assert plant.mounting_height == 1.5 + assert (plant.dc_ohmic_loss, plant.ac_ohmic_loss) == (0.007, 0.010) + assert plant.bifacial_shade_loss == 0.100 + assert plant.transformer_fixed_loss == 0 + + def test_explicit_values_are_not_overwritten(self): + plant = PVSystem(latitude=45.0, longitude=10.0, mounting_height=2.0, dc_ohmic_loss=0.02) + + plant.mounting = "Tracker" + plant.inverter_type = "String" + + assert plant.mounting_height == 2.0 + assert plant.dc_ohmic_loss == 0.02 + assert plant.ac_ohmic_loss == 0.010 # still auto-filled + + def test_assigned_value_stops_autofill_and_none_restores_it(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + plant.mounting_height = 3.0 + plant.mounting = "Tracker" + assert plant.mounting_height == 3.0 + + plant.mounting_height = None + assert plant.mounting_height == 1.5 + plant.mounting = "Fixed" + assert plant.mounting_height == 0.7 + + def test_make_copy_keeps_autofill_tracking(self): + plant = PVSystem(latitude=45.0, longitude=10.0, dc_ohmic_loss=0.02) + + copy = plant.make_copy() + copy.mounting = "Tracker" + copy.inverter_type = "String" + + assert copy.mounting_height == 1.5 + assert copy.dc_ohmic_loss == 0.02 + assert copy.ac_ohmic_loss == 0.010 + + def test_invalid_transformer_stages_on_reassignment_raises(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + with pytest.raises(ValueError, match="transformer_stages"): + plant.transformer_stages = 2 + + def test_payload_cache_invalidated_on_config_change(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + plant.payload = "cached" + plant.tilt = 20.0 + assert plant.payload is None + + plant.payload = "cached" + plant.mounting = "tracker" + assert plant.payload is None + + def test_payload_cache_kept_when_only_outputs_change(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + plant.payload = "cached" + plant.results = object() + plant.design_summary = {} + + assert plant.payload == "cached" + + def test_payload_cache_invalidated_on_equipment_files_change(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + for update in ( + lambda: setattr(plant, "pan_files", {"m": "m.PAN"}), + lambda: setattr(plant, "ond_files", {"i": "i.OND"}), + lambda: plant.add_pan_files({"m2": "m2.PAN"}), + lambda: plant.add_ond_files({"i2": "i2.OND"}), + ): + plant.payload = "cached" + update() + assert plant.payload is None + + def test_payload_loaded_from_file_is_kept(self, tmp_path): + plant = PVSystem(latitude=45.0, longitude=10.0) + plant.payload = "cached" + plant.to_file(tmp_path / "plant.json") + + assert PVSystem.from_file(tmp_path / "plant.json").payload == "cached" + + def test_pvsystem_inverter_type_none_defaults_to_central(self): + plant = PVSystem(latitude=45.0, longitude=10.0, inverter_type=None) + + assert plant.inverter_type is InverterType.CENTRAL + class TestPVSystemTechnologyProperties: """Test cases for PVSystem technology properties.""" From f4745b8c06e003023e8d87eaad9b054f7aaca3e6 Mon Sep 17 00:00:00 2001 From: Javier Lopez Lorente Date: Tue, 13 Oct 2026 15:51:18 +0200 Subject: [PATCH 02/12] Refactor to recompute derived inputs from defaults post_init and in cached payload --- solarfarmer/models/pvsystem/pvsystem.py | 189 +++++++++++++----------- 1 file changed, 106 insertions(+), 83 deletions(-) diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index 2247e5c..e8e252c 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -2,8 +2,9 @@ import copy as copy_module import json -from collections.abc import Iterable, Mapping, Sequence +from collections.abc import Callable, Iterable, Mapping, Sequence from dataclasses import asdict, dataclass, field +from enum import Enum from pathlib import Path from typing import Any @@ -64,6 +65,52 @@ _logger = get_logger("models.pvsystem") +# Attributes coerced to their enum on every assignment (case-insensitive for strings). +_ENUM_FIELDS: dict[str, type[Enum]] = { + "mounting": MountingType, + "module_orientation": OrientationType, + "inverter_type": InverterType, +} + +_VALID_TRANSFORMER_STAGES = (0, 1) +_TRANSFORMER_KEYS = {0: "Ideal", 1: "FixedType1"} + + +def _bifacial_params(plant: PVSystem) -> dict[str, float]: + return BIFACIAL_DICT[True][plant.mounting.value] if plant.bifacial else BIFACIAL_DICT[False] + + +# Fields with a default derived from other fields; recomputed only while still auto-filled. +_DERIVED_DEFAULTS: dict[str, Callable[[PVSystem], float]] = { + "mounting_height": lambda p: ( + HEIGHT_FROM_GROUND_FIXED if p.mounting == MountingType.FIXED else HEIGHT_FROM_GROUND_TRACKER + ), + "dc_ohmic_loss": lambda p: OHMIC_DICT[p.inverter_type.value]["dc"], + "ac_ohmic_loss": lambda p: OHMIC_DICT[p.inverter_type.value]["ac"], + "bifacial_transmission": lambda p: _bifacial_params(p)["transmission"], + "bifacial_shade_loss": lambda p: _bifacial_params(p)["shade"], + "bifacial_mismatch_loss": lambda p: _bifacial_params(p)["mismatch"], + "transformer_fixed_loss": lambda p: TRANSFORMER_DICT[_TRANSFORMER_KEYS[p.transformer_stages]][ + "fixed" + ], + "transformer_variable_loss": lambda p: TRANSFORMER_DICT[ + _TRANSFORMER_KEYS[p.transformer_stages] + ]["variable"], +} + +_BIFACIAL_FIELDS = ("bifacial_transmission", "bifacial_shade_loss", "bifacial_mismatch_loss") + +# Outputs and bookkeeping: changing them must not invalidate the cached payload. +_NON_CONFIG_FIELDS = frozenset({"payload", "results", "design_summary", "_auto_filled"}) + +# Input field -> derived fields to refresh when it is reassigned. +_DERIVED_DEPENDENTS: dict[str, tuple[str, ...]] = { + "mounting": ("mounting_height", *_BIFACIAL_FIELDS), + "bifacial": _BIFACIAL_FIELDS, + "inverter_type": ("dc_ohmic_loss", "ac_ohmic_loss"), + "transformer_stages": ("transformer_fixed_loss", "transformer_variable_loss"), +} + PathLike = str | Path @@ -487,6 +534,7 @@ def pan_files(self, mapping: Mapping[str, PathLike] | Sequence[PathLike]) -> Non contains paths whose stems collide (duplicate filenames). """ self._pan_files.clear() + self.payload = None if isinstance(mapping, Mapping): for name, p in mapping.items(): key = str(name).strip() @@ -524,6 +572,7 @@ def add_pan_files(self, mapping: Mapping[str, PathLike]) -> PVSystem: if not key: raise ValueError("Module name cannot be empty") self._pan_files[key] = Path(p) + self.payload = None return self @property @@ -552,6 +601,7 @@ def ond_files(self, mapping: Mapping[str, PathLike] | Sequence[PathLike]) -> Non contains paths whose stems collide (duplicate filenames). """ self._ond_files.clear() + self.payload = None if isinstance(mapping, Mapping): for name, p in mapping.items(): key = str(name).strip() @@ -589,6 +639,7 @@ def add_ond_files(self, mapping: Mapping[str, PathLike]) -> PVSystem: if not key: raise ValueError("Inverter name cannot be empty") self._ond_files[key] = Path(p) + self.payload = None return self @property @@ -601,34 +652,62 @@ def ond_file_map(self) -> dict[str, Path]: """A (shallow) copy of inverter -> OND file mapping.""" return dict(self._ond_files) + def __setattr__(self, name: str, value: Any) -> None: + enum_cls = _ENUM_FIELDS.get(name) + if enum_cls is not None and not isinstance(value, enum_cls): + # inverter_type=None means "use the default". + if value is None and name == "inverter_type": + value = InverterType.CENTRAL + else: + try: + value = enum_cls(value) + except ValueError as err: + raise ValueError( + f"{value!r} is not a valid {enum_cls.__name__} for '{name}'. " + f"Must be one of: {', '.join(e.value for e in enum_cls)} " + "(case-insensitive)." + ) from err + elif name == "transformer_stages" and value not in _VALID_TRANSFORMER_STAGES: + raise ValueError( + f"Invalid transformer_stages: {value}. " + f"Must be one of: {list(_VALID_TRANSFORMER_STAGES)}" + ) + + super().__setattr__(name, value) + + # Not yet created while the dataclass __init__ is still assigning fields. + auto_filled = self.__dict__.get("_auto_filled") + if auto_filled is None: + return + if name not in _NON_CONFIG_FIELDS: + object.__setattr__(self, "payload", None) + if name in _DERIVED_DEFAULTS: + if value is None: + self._refresh_derived(name) # None resets the field to its default + else: + auto_filled.discard(name) # explicit user value from now on + for dependent in _DERIVED_DEPENDENTS.get(name, ()): + self._refresh_derived(dependent) + + def _refresh_derived(self, name: str) -> None: + """Set a derived default if the field is empty or was previously auto-filled.""" + if getattr(self, name) is None or name in self._auto_filled: + # object.__setattr__ bypasses the hook so the field stays marked as auto-filled. + object.__setattr__(self, name, _DERIVED_DEFAULTS[name](self)) + self._auto_filled.add(name) + def __post_init__(self) -> None: """Post-initialization processing and validation.""" - # Convert string values to enums if necessary - # (useful when loading JSON via `from_file()`) - if isinstance(self.mounting, str): - self.mounting = MountingType(self.mounting) - if isinstance(self.module_orientation, str): - self.module_orientation = OrientationType(self.module_orientation) - if isinstance(self.inverter_type, str): - self.inverter_type = InverterType(self.inverter_type) - - # Initialize mounting_height based on mounting type if not explicitly set - if self.mounting == MountingType.FIXED: - if self.mounting_height is None or not hasattr(self, "mounting_height"): - self.mounting_height = HEIGHT_FROM_GROUND_FIXED - elif self.mounting == MountingType.TRACKER: - if self.mounting_height is None or not hasattr(self, "mounting_height"): - self.mounting_height = HEIGHT_FROM_GROUND_TRACKER - else: - raise ValueError( - f"Invalid mounting type: '{self.mounting}'. " - f"Must be one of: {', '.join([e.value for e in MountingType])}" - ) + # Enum fields and transformer_stages are already validated by __setattr__. + self._auto_filled: set[str] = set() + for name in _DERIVED_DEFAULTS: + self._refresh_derived(name) # Initialize modules_across with default if not set if self.modules_across is None: - self.modules_across = NUMBER_OF_MODULES_HIGH + # Bypass the hook so a payload loaded via from_file() is not discarded. + object.__setattr__(self, "modules_across", NUMBER_OF_MODULES_HIGH) # Validate modules_across if not isinstance(self.modules_across, int) or self.modules_across <= 0: @@ -636,65 +715,6 @@ def __post_init__(self) -> None: f"modules_across must be a positive integer, got {self.modules_across}." ) - # Initialize and validate inverter_type - if self.inverter_type is None: - self.inverter_type = InverterType( - InverterType.CENTRAL - ) # Default to 'Central' if not set - - try: - ( - self.inverter_type.value - if isinstance(self.inverter_type, InverterType) - else self.inverter_type - ) - except (AttributeError, ValueError) as err: - raise ValueError( - f"Invalid inverter_type: '{self.inverter_type}'. " - f"Must be one of: {', '.join([e.value for e in InverterType])}" - ) from err - - # Initialize dc_ohmic_loss and ac_ohmic_loss based on inverter_type - if self.dc_ohmic_loss is None: - self.dc_ohmic_loss = OHMIC_DICT[self.inverter_type.value]["dc"] - - if self.ac_ohmic_loss is None: - self.ac_ohmic_loss = OHMIC_DICT[self.inverter_type.value]["ac"] - - # Initialize bifacial parameters based on bifacial flag and mounting type - if self.bifacial: - bifacial_params = BIFACIAL_DICT[True][self.mounting.value] - else: - bifacial_params = BIFACIAL_DICT[False] - - if self.bifacial_transmission is None: - self.bifacial_transmission = bifacial_params["transmission"] - - if self.bifacial_shade_loss is None: - self.bifacial_shade_loss = bifacial_params["shade"] - - if self.bifacial_mismatch_loss is None: - self.bifacial_mismatch_loss = bifacial_params["mismatch"] - - # Initialize and validate transformer_stages - valid_transformer_stages = {0, 1} - if self.transformer_stages not in valid_transformer_stages: - raise ValueError( - f"Invalid transformer_stages: {self.transformer_stages}. " - f"Must be one of: {sorted(valid_transformer_stages)}" - ) - - # Map transformer_stages to TRANSFORMER_DICT keys and initialize losses - transformer_key_map = {0: "Ideal", 1: "FixedType1"} - transformer_type = transformer_key_map[self.transformer_stages] - transformer_params = TRANSFORMER_DICT[transformer_type] - - if self.transformer_fixed_loss is None: - self.transformer_fixed_loss = transformer_params["fixed"] - - if self.transformer_variable_loss is None: - self.transformer_variable_loss = transformer_params["variable"] - # Validate location if self.latitude is not None and not (-90.0 <= float(self.latitude) <= 90.0): raise ValueError("latitude must be within [-90, 90] degrees") @@ -763,7 +783,10 @@ def make_copy(self) -> PVSystem: plant_dict["name"] = f"Copy of {original_name}" # Create and return new PVSystem instance - return PVSystem(**plant_dict) + new_plant = PVSystem(**plant_dict) + # Values were passed in explicitly above; keep the original's auto-filled tracking. + new_plant._auto_filled = set(self._auto_filled) + return new_plant def describe(self, verbose=False) -> None: """Print a comprehensive summary of all PV plant configuration properties. From d9dd7b72f006e7e72521ae53efaeed79901c6689 Mon Sep 17 00:00:00 2001 From: Javier Lopez Lorente Date: Tue, 13 Oct 2026 16:15:28 +0200 Subject: [PATCH 03/12] Fix formatting JSON export via PVSystem.payload_to_file() --- solarfarmer/models/pvsystem/pvsystem.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index 2247e5c..7039ad6 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -1011,12 +1011,12 @@ def payload_to_file(self, file_path: PathLike) -> None: else: payload_dict = self.payload + # construct_plant returns compact JSON; re-serialize so the file is indented + if isinstance(payload_dict, str): + payload_dict = json.loads(payload_dict) + with path.open("w", encoding="utf-8") as f: - # If payload is a string (JSON), write directly; if dict, use json.dump - if isinstance(payload_dict, str): - f.write(payload_dict) - else: - json.dump(payload_dict, f, indent=4) + json.dump(payload_dict, f, indent=4) _logger.debug("PVSystem payload saved to %s", path) return None From 72d2fcc9cfdb165ff97d781bd6ca7535efae0f20 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Fri, 16 Oct 2026 12:46:25 -0700 Subject: [PATCH 04/12] Make PVSystem.payload an output-only snapshot Always rebuild the payload from current plant configuration, avoiding stale payloads after edits. Store the last built or sent JSON for inspection, but omit it from saved configurations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- solarfarmer/models/pvsystem/pvsystem.py | 61 ++++++---------- tests/test_pvsystem.py | 92 ++++++++++++++++--------- 2 files changed, 79 insertions(+), 74 deletions(-) diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index e8e252c..52bb918 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -100,9 +100,6 @@ def _bifacial_params(plant: PVSystem) -> dict[str, float]: _BIFACIAL_FIELDS = ("bifacial_transmission", "bifacial_shade_loss", "bifacial_mismatch_loss") -# Outputs and bookkeeping: changing them must not invalidate the cached payload. -_NON_CONFIG_FIELDS = frozenset({"payload", "results", "design_summary", "_auto_filled"}) - # Input field -> derived fields to refresh when it is reassigned. _DERIVED_DEPENDENTS: dict[str, tuple[str, ...]] = { "mounting": ("mounting_height", *_BIFACIAL_FIELDS), @@ -357,8 +354,8 @@ class PVSystem: # Results results: CalculationResults = None - # payload dictionary - payload: dict[str, Any] = None + # Last payload built or sent (JSON string); output only, never read back. + payload: str | None = None # design summary dictionary design_summary: dict[str, Any] = None @@ -534,7 +531,6 @@ def pan_files(self, mapping: Mapping[str, PathLike] | Sequence[PathLike]) -> Non contains paths whose stems collide (duplicate filenames). """ self._pan_files.clear() - self.payload = None if isinstance(mapping, Mapping): for name, p in mapping.items(): key = str(name).strip() @@ -572,7 +568,6 @@ def add_pan_files(self, mapping: Mapping[str, PathLike]) -> PVSystem: if not key: raise ValueError("Module name cannot be empty") self._pan_files[key] = Path(p) - self.payload = None return self @property @@ -601,7 +596,6 @@ def ond_files(self, mapping: Mapping[str, PathLike] | Sequence[PathLike]) -> Non contains paths whose stems collide (duplicate filenames). """ self._ond_files.clear() - self.payload = None if isinstance(mapping, Mapping): for name, p in mapping.items(): key = str(name).strip() @@ -639,7 +633,6 @@ def add_ond_files(self, mapping: Mapping[str, PathLike]) -> PVSystem: if not key: raise ValueError("Inverter name cannot be empty") self._ond_files[key] = Path(p) - self.payload = None return self @property @@ -675,12 +668,9 @@ def __setattr__(self, name: str, value: Any) -> None: super().__setattr__(name, value) - # Not yet created while the dataclass __init__ is still assigning fields. auto_filled = self.__dict__.get("_auto_filled") if auto_filled is None: return - if name not in _NON_CONFIG_FIELDS: - object.__setattr__(self, "payload", None) if name in _DERIVED_DEFAULTS: if value is None: self._refresh_derived(name) # None resets the field to its default @@ -764,6 +754,7 @@ def make_copy(self) -> PVSystem: # Explicitly clear results in the copy plant_dict["results"] = None + plant_dict["payload"] = None # Deep copy mutable fields to avoid shared references mutable_fields = [ @@ -946,6 +937,7 @@ def to_file(self, file_path: PathLike) -> None: including metadata, location, plant definition, effects, and auxiliary files. - The results field is not included in the saved configuration, as it is typically generated after running the energy calculation and may contain complex data structures. + - The payload snapshot is not included; it is rebuilt from the saved configuration when needed. """ path = Path(file_path) path.parent.mkdir(parents=True, exist_ok=True) # Ensure parent directories exist @@ -953,6 +945,7 @@ def to_file(self, file_path: PathLike) -> None: # Convert PVSystem to dictionary and exclude results for saving plant_dict = asdict(self) plant_dict.pop("results", None) # Remove results from saved config + plant_dict.pop("payload", None) # Payload is an output snapshot, not plant configuration. # Convert Path objects to strings for JSON compatibility for key in ("_weather_file", "_horizon_file"): @@ -1004,6 +997,9 @@ def from_file(cls, file_path: PathLike) -> PVSystem: if plant_dict.get(key): plant_dict[key] = {k: Path(v) for k, v in plant_dict[key].items()} + # Ignore payloads saved by older SDK versions; they may no longer match the configuration. + plant_dict.pop("payload", None) + # Create PVSystem instance from loaded dictionary return cls(**plant_dict) @@ -1024,22 +1020,15 @@ def payload_to_file(self, file_path: PathLike) -> None: ----- - This method constructs the payload using the current PVSystem configuration and saves it as JSON. - The payload is what is sent to the SolarFarmer API for energy calculation. - - If the payload has already been constructed and stored in self.payload, it will save that instead of reconstructing. + - The payload is always rebuilt from the current configuration. """ path = Path(file_path) path.parent.mkdir(parents=True, exist_ok=True) # Ensure parent directories exist - if self.payload is None: - payload_dict = construct_plant(self) - else: - payload_dict = self.payload + payload = self.produce_payload() with path.open("w", encoding="utf-8") as f: - # If payload is a string (JSON), write directly; if dict, use json.dump - if isinstance(payload_dict, str): - f.write(payload_dict) - else: - json.dump(payload_dict, f, indent=4) + f.write(payload) _logger.debug("PVSystem payload saved to %s", path) return None @@ -1346,26 +1335,21 @@ def set_inverter_from_rcl( _logger.info("Inverter '%s' set from RCL (%d bytes)", inverter_name, len(content)) return item - def produce_payload(self) -> dict[str, Any]: - """Construct and return the payload dictionary for the SolarFarmer API based on the current PVSystem configuration. + def produce_payload(self) -> str: + """Build and return the JSON payload for the current PVSystem configuration. Returns ------- - dict[str, Any] - A dictionary representing the payload to be sent to the SolarFarmer API for energy calculation. + str + A JSON string representing the payload to be sent to the SolarFarmer API. Notes ----- - - This method constructs the payload using the current configuration of the PVSystem instance. - - The payload includes all necessary information about the plant design, location, losses, and other parameters required by the API. - - If the payload has already been constructed and stored in self.payload, it will return that instead of reconstructing. + The payload is always rebuilt from the current configuration and stored in + ``self.payload`` for inspection. """ - if self.payload is None: - payload = construct_plant(self) - self.payload = payload # Cache the constructed payload for future use - return payload - else: - return self.payload + self.payload = construct_plant(self) + return self.payload def run_energy_calculation( self, @@ -1421,11 +1405,8 @@ def run_energy_calculation( """ from ...endpoint_modelchains import run_energy_calculation - # Build the JSON payload from the PVSystem configuration - if self.payload is None: - builder = construct_plant(self) - else: - builder = self.payload + # Always build from the current configuration and retain what was sent for inspection. + builder = self.produce_payload() # Make API call via modelchain function results = run_energy_calculation( diff --git a/tests/test_pvsystem.py b/tests/test_pvsystem.py index a1d9d16..09dfb42 100644 --- a/tests/test_pvsystem.py +++ b/tests/test_pvsystem.py @@ -268,50 +268,74 @@ def test_invalid_transformer_stages_on_reassignment_raises(self): with pytest.raises(ValueError, match="transformer_stages"): plant.transformer_stages = 2 - def test_payload_cache_invalidated_on_config_change(self): - plant = PVSystem(latitude=45.0, longitude=10.0) + def test_pvsystem_inverter_type_none_defaults_to_central(self): + plant = PVSystem(latitude=45.0, longitude=10.0, inverter_type=None) - plant.payload = "cached" - plant.tilt = 20.0 - assert plant.payload is None + assert plant.inverter_type is InverterType.CENTRAL - plant.payload = "cached" - plant.mounting = "tracker" - assert plant.payload is None - def test_payload_cache_kept_when_only_outputs_change(self): - plant = PVSystem(latitude=45.0, longitude=10.0) +class TestPVSystemPayloadSnapshot: + @staticmethod + def _plant(sample_data_dir): + data_dir = Path(sample_data_dir) / "Inputs_Bern_2D_racks" + plant = PVSystem( + latitude=46.9, + longitude=7.4, + dc_capacity_MW=5.0, + ac_capacity_MW=4.0, + tilt=20.0, + ) + plant.pan_files = {"Module": data_dir / "CanadianSolar_CS6U-330M_APP.PAN"} + plant.ond_files = {"Inverter": data_dir / "Sungrow_SG125HV_APP.OND"} + plant.weather_file = data_dir / "Bern-hour.dat" + return plant - plant.payload = "cached" - plant.results = object() - plant.design_summary = {} + @staticmethod + def _tilt_values(value, result): + if isinstance(value, dict): + for key, nested_value in value.items(): + if key == "tilt": + result.add(nested_value) + TestPVSystemPayloadSnapshot._tilt_values(nested_value, result) + elif isinstance(value, list): + for nested_value in value: + TestPVSystemPayloadSnapshot._tilt_values(nested_value, result) + return result - assert plant.payload == "cached" + def test_produce_payload_reflects_edits(self, sample_data_dir): + plant = self._plant(sample_data_dir) - def test_payload_cache_invalidated_on_equipment_files_change(self): - plant = PVSystem(latitude=45.0, longitude=10.0) + first = json.loads(plant.produce_payload()) + plant.tilt = 35.0 + second = json.loads(plant.produce_payload()) - for update in ( - lambda: setattr(plant, "pan_files", {"m": "m.PAN"}), - lambda: setattr(plant, "ond_files", {"i": "i.OND"}), - lambda: plant.add_pan_files({"m2": "m2.PAN"}), - lambda: plant.add_ond_files({"i2": "i2.OND"}), - ): - plant.payload = "cached" - update() - assert plant.payload is None - - def test_payload_loaded_from_file_is_kept(self, tmp_path): - plant = PVSystem(latitude=45.0, longitude=10.0) - plant.payload = "cached" - plant.to_file(tmp_path / "plant.json") + assert self._tilt_values(first, set()) == {20.0} + assert self._tilt_values(second, set()) == {35.0} - assert PVSystem.from_file(tmp_path / "plant.json").payload == "cached" + def test_make_copy_resets_payload(self, sample_data_dir): + plant = self._plant(sample_data_dir) + plant.produce_payload() - def test_pvsystem_inverter_type_none_defaults_to_central(self): - plant = PVSystem(latitude=45.0, longitude=10.0, inverter_type=None) + assert plant.make_copy().payload is None - assert plant.inverter_type is InverterType.CENTRAL + def test_to_file_omits_payload(self, sample_data_dir, tmp_path): + plant = self._plant(sample_data_dir) + plant.produce_payload() + file_path = tmp_path / "plant.json" + + plant.to_file(file_path) + + assert "payload" not in json.loads(file_path.read_text()) + + def test_from_file_ignores_legacy_payload(self, tmp_path): + plant = PVSystem(latitude=45.0, longitude=10.0) + file_path = tmp_path / "plant.json" + plant.to_file(file_path) + plant_data = json.loads(file_path.read_text()) + plant_data["payload"] = "stale" + file_path.write_text(json.dumps(plant_data)) + + assert PVSystem.from_file(file_path).payload is None class TestPVSystemTechnologyProperties: From d6d610ab3ea69e15815cec0da1ada17ee042fd87 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Fri, 16 Oct 2026 13:11:58 -0700 Subject: [PATCH 05/12] Resolve derived PVSystem defaults at build time Keep unset derived inputs as None and resolve them from final plant settings on a copy during payload construction and description. This makes the documented assign-after-construction workflow order-independent and prevents design calculations from overwriting the user's capacities or GCR. Reduce __setattr__ to enum coercion and remove hidden auto-fill tracking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- solarfarmer/models/pvsystem/pvsystem.py | 181 +++++++++++----------- tests/test_construct_plant.py | 8 +- tests/test_pvsystem.py | 191 +++++++++++++++++------- 3 files changed, 226 insertions(+), 154 deletions(-) diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index 52bb918..75d4fd7 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -2,8 +2,8 @@ import copy as copy_module import json -from collections.abc import Callable, Iterable, Mapping, Sequence -from dataclasses import asdict, dataclass, field +from collections.abc import Iterable, Mapping, Sequence +from dataclasses import asdict, dataclass, field, replace from enum import Enum from pathlib import Path from typing import Any @@ -72,41 +72,28 @@ "inverter_type": InverterType, } -_VALID_TRANSFORMER_STAGES = (0, 1) -_TRANSFORMER_KEYS = {0: "Ideal", 1: "FixedType1"} - - -def _bifacial_params(plant: PVSystem) -> dict[str, float]: - return BIFACIAL_DICT[True][plant.mounting.value] if plant.bifacial else BIFACIAL_DICT[False] - - -# Fields with a default derived from other fields; recomputed only while still auto-filled. -_DERIVED_DEFAULTS: dict[str, Callable[[PVSystem], float]] = { - "mounting_height": lambda p: ( - HEIGHT_FROM_GROUND_FIXED if p.mounting == MountingType.FIXED else HEIGHT_FROM_GROUND_TRACKER - ), - "dc_ohmic_loss": lambda p: OHMIC_DICT[p.inverter_type.value]["dc"], - "ac_ohmic_loss": lambda p: OHMIC_DICT[p.inverter_type.value]["ac"], - "bifacial_transmission": lambda p: _bifacial_params(p)["transmission"], - "bifacial_shade_loss": lambda p: _bifacial_params(p)["shade"], - "bifacial_mismatch_loss": lambda p: _bifacial_params(p)["mismatch"], - "transformer_fixed_loss": lambda p: TRANSFORMER_DICT[_TRANSFORMER_KEYS[p.transformer_stages]][ - "fixed" - ], - "transformer_variable_loss": lambda p: TRANSFORMER_DICT[ - _TRANSFORMER_KEYS[p.transformer_stages] - ]["variable"], -} -_BIFACIAL_FIELDS = ("bifacial_transmission", "bifacial_shade_loss", "bifacial_mismatch_loss") +def _default_derived_values(plant: PVSystem) -> dict[str, float]: + """Defaults for the derived inputs, computed from the plant's current inputs.""" + bifacial = BIFACIAL_DICT[True][plant.mounting.value] if plant.bifacial else BIFACIAL_DICT[False] + transformer_key = {0: "Ideal", 1: "FixedType1"}[plant.transformer_stages] + transformer = TRANSFORMER_DICT[transformer_key] + ohmic = OHMIC_DICT[plant.inverter_type.value] + return { + "mounting_height": ( + HEIGHT_FROM_GROUND_FIXED + if plant.mounting == MountingType.FIXED + else HEIGHT_FROM_GROUND_TRACKER + ), + "dc_ohmic_loss": ohmic["dc"], + "ac_ohmic_loss": ohmic["ac"], + "bifacial_transmission": bifacial["transmission"], + "bifacial_shade_loss": bifacial["shade"], + "bifacial_mismatch_loss": bifacial["mismatch"], + "transformer_fixed_loss": transformer["fixed"], + "transformer_variable_loss": transformer["variable"], + } -# Input field -> derived fields to refresh when it is reassigned. -_DERIVED_DEPENDENTS: dict[str, tuple[str, ...]] = { - "mounting": ("mounting_height", *_BIFACIAL_FIELDS), - "bifacial": _BIFACIAL_FIELDS, - "inverter_type": ("dc_ohmic_loss", "ac_ohmic_loss"), - "transformer_stages": ("transformer_fixed_loss", "transformer_variable_loss"), -} PathLike = str | Path @@ -660,44 +647,14 @@ def __setattr__(self, name: str, value: Any) -> None: f"Must be one of: {', '.join(e.value for e in enum_cls)} " "(case-insensitive)." ) from err - elif name == "transformer_stages" and value not in _VALID_TRANSFORMER_STAGES: - raise ValueError( - f"Invalid transformer_stages: {value}. " - f"Must be one of: {list(_VALID_TRANSFORMER_STAGES)}" - ) - super().__setattr__(name, value) - auto_filled = self.__dict__.get("_auto_filled") - if auto_filled is None: - return - if name in _DERIVED_DEFAULTS: - if value is None: - self._refresh_derived(name) # None resets the field to its default - else: - auto_filled.discard(name) # explicit user value from now on - for dependent in _DERIVED_DEPENDENTS.get(name, ()): - self._refresh_derived(dependent) - - def _refresh_derived(self, name: str) -> None: - """Set a derived default if the field is empty or was previously auto-filled.""" - if getattr(self, name) is None or name in self._auto_filled: - # object.__setattr__ bypasses the hook so the field stays marked as auto-filled. - object.__setattr__(self, name, _DERIVED_DEFAULTS[name](self)) - self._auto_filled.add(name) - def __post_init__(self) -> None: """Post-initialization processing and validation.""" - # Enum fields and transformer_stages are already validated by __setattr__. - self._auto_filled: set[str] = set() - for name in _DERIVED_DEFAULTS: - self._refresh_derived(name) - # Initialize modules_across with default if not set if self.modules_across is None: - # Bypass the hook so a payload loaded via from_file() is not discarded. - object.__setattr__(self, "modules_across", NUMBER_OF_MODULES_HIGH) + self.modules_across = NUMBER_OF_MODULES_HIGH # Validate modules_across if not isinstance(self.modules_across, int) or self.modules_across <= 0: @@ -705,6 +662,14 @@ def __post_init__(self) -> None: f"modules_across must be a positive integer, got {self.modules_across}." ) + # Validate transformer stages. + valid_transformer_stages = {0, 1} + if self.transformer_stages not in valid_transformer_stages: + raise ValueError( + f"Invalid transformer_stages: {self.transformer_stages}. " + f"Must be one of: {sorted(valid_transformer_stages)}" + ) + # Validate location if self.latitude is not None and not (-90.0 <= float(self.latitude) <= 90.0): raise ValueError("latitude must be within [-90, 90] degrees") @@ -725,6 +690,39 @@ def __post_init__(self) -> None: if v is None: raise ValueError("Horizon angles must not contain None") + def with_defaults_applied(self) -> PVSystem: + """Return a copy with unset derived inputs filled from the current plant settings. + + Derived inputs left as ``None`` are resolved from the final values of + ``mounting``, ``bifacial``, ``inverter_type``, and ``transformer_stages``. + Explicit values, including ``0.0``, are preserved. This method does not + modify the current plant. + + Returns + ------- + PVSystem + A copy of this plant with all unset derived inputs resolved. + + Raises + ------ + ValueError + If the plant configuration, including ``transformer_stages``, is invalid. + + Examples + -------- + >>> plant = PVSystem(latitude=45.0, longitude=10.0, mounting="Tracker") + >>> resolved = plant.with_defaults_applied() + >>> resolved.mounting_height + 1.5 + >>> plant.mounting_height is None + True + """ + resolved = replace(self, results=None, payload=None, design_summary=None) + for name, default in _default_derived_values(resolved).items(): + if getattr(resolved, name) is None: + setattr(resolved, name, default) + return resolved + def make_copy(self) -> PVSystem: """Create a deep copy of the PVSystem instance with results cleared. @@ -774,10 +772,7 @@ def make_copy(self) -> PVSystem: plant_dict["name"] = f"Copy of {original_name}" # Create and return new PVSystem instance - new_plant = PVSystem(**plant_dict) - # Values were passed in explicitly above; keep the original's auto-filled tracking. - new_plant._auto_filled = set(self._auto_filled) - return new_plant + return PVSystem(**plant_dict) def describe(self, verbose=False) -> None: """Print a comprehensive summary of all PV plant configuration properties. @@ -785,6 +780,7 @@ def describe(self, verbose=False) -> None: Displays all settings organized by category: metadata, location, capacity, array configuration, mounting, inverter/transformer, losses, and files. """ + plant = self.with_defaults_applied() print("\n" + "=" * 70) print("PV PLANT SUMMARY") print("=" * 70) @@ -828,16 +824,16 @@ def describe(self, verbose=False) -> None: print(f"Module Orientation: {self.module_orientation}") print(f"Modules Across: {self.modules_across}") if verbose: - print(f"Mounting Height: {self.mounting_height} m") + print(f"Mounting Height: {plant.mounting_height} m") print(f"Flush Mount: {self.flush_mount}") # Bifacial print("\n--- BIFACIAL PARAMETERS ---") print(f"Bifacial Modules: {self.bifacial}") if verbose: - print(f" - Transmission Factor: {self.bifacial_transmission}") - print(f" - Shade Loss Factor: {self.bifacial_shade_loss}") - print(f" - Mismatch Loss Factor: {self.bifacial_mismatch_loss}") + print(f" - Transmission Factor: {plant.bifacial_transmission}") + print(f" - Shade Loss Factor: {plant.bifacial_shade_loss}") + print(f" - Mismatch Loss Factor: {plant.bifacial_mismatch_loss}") # Inverter & Transformer print("\n--- INVERTER & TRANSFORMER ---") @@ -853,13 +849,13 @@ def describe(self, verbose=False) -> None: if verbose: # Losses print("\n--- LOSSES ---") - print(f"DC Ohmic Loss: {self.dc_ohmic_loss} (per unit)") - print(f"AC Ohmic Loss: {self.ac_ohmic_loss} (per unit)") + print(f"DC Ohmic Loss: {plant.dc_ohmic_loss} (per unit)") + print(f"AC Ohmic Loss: {plant.ac_ohmic_loss} (per unit)") print(f"Module Mismatch Loss: {self.module_mismatch} (per unit)") print(f"Module Quality Factor: {self.module_quality_factor} (per unit)") print(f"LID Loss: {self.lid_loss} (per unit)") - print(f"Transformer Fixed Loss: {self.transformer_fixed_loss} (per unit)") - print(f"Transformer Variable Loss: {self.transformer_variable_loss} (per unit)") + print(f"Transformer Fixed Loss: {plant.transformer_fixed_loss} (per unit)") + print(f"Transformer Variable Loss: {plant.transformer_variable_loss} (per unit)") print(f"Auxiliary Loss (Fixed Factor): {self.aux_loss_fixed_factor} (per unit)") print(f"Auxiliary Loss (Power): {self.aux_loss_power} W") print(f"Auxiliary Loss Applied at Night: {self.aux_loss_apply_at_night}") @@ -1436,43 +1432,44 @@ def construct_plant(pvplant: PVSystem) -> str: Returns a JSON string ready to be sent to the SolarFarmer API. """ + plant = pvplant.with_defaults_applied() + # Location - location = Location( - longitude=pvplant.longitude, latitude=pvplant.latitude, altitude=pvplant.altitude - ) + location = Location(longitude=plant.longitude, latitude=plant.latitude, altitude=plant.altitude) # PV plant - pv_plant, pan_file_supplements = design_plant(pvplant=pvplant) + pv_plant, pan_file_supplements = design_plant(pvplant=plant) # Energy calculation options - has_horizon = bool(pvplant.horizon_elevation_angles) + has_horizon = bool(plant.horizon_elevation_angles) calculation_options = EnergyCalculationOptions( diffuse_model=DiffuseModel.PEREZ, include_horizon=has_horizon, ) # update non-default energy calculation options - calculation_options.return_loss_tree_time_series_results = pvplant.generate_loss_tree_timeseries + calculation_options.return_loss_tree_time_series_results = plant.generate_loss_tree_timeseries calculation_options.return_pv_syst_format_time_series_results = ( - pvplant.generate_pvsyst_format_timeseries + plant.generate_pvsyst_format_timeseries ) - calculation_options.apply_spectral_mismatch_modifier = pvplant.enable_spectral_modeling - calculation_options.calculate_dhi = pvplant.calculate_dhi_from_ghi - calculation_options.system_availability_loss = pvplant.plant_unavailability - calculation_options.grid_availability_loss = pvplant.grid_unavailability + calculation_options.apply_spectral_mismatch_modifier = plant.enable_spectral_modeling + calculation_options.calculate_dhi = plant.calculate_dhi_from_ghi + calculation_options.system_availability_loss = plant.plant_unavailability + calculation_options.grid_availability_loss = plant.grid_unavailability # Build the full inputs model inputs = EnergyCalculationInputs( location=location, pv_plant=pv_plant, - monthly_albedo=MonthlyAlbedo(values=pvplant.albedo), + monthly_albedo=MonthlyAlbedo(values=plant.albedo), energy_calculation_options=calculation_options, - horizon_azimuths=pvplant.horizon_azimuth_angles or None, - horizon_angles=pvplant.horizon_elevation_angles or None, + horizon_azimuths=plant.horizon_azimuth_angles or None, + horizon_angles=plant.horizon_elevation_angles or None, pan_file_supplements=pan_file_supplements or None, ) # TODO: Integrate OND file supplements into payload construction + pvplant.design_summary = plant.design_summary return inputs.model_dump_json(by_alias=True, exclude_none=True) diff --git a/tests/test_construct_plant.py b/tests/test_construct_plant.py index 926ea78..09a85e7 100644 --- a/tests/test_construct_plant.py +++ b/tests/test_construct_plant.py @@ -98,7 +98,7 @@ class TestDesignPlant: """Tests for design_plant() → (PVPlant, supplements dict).""" def test_returns_pvplant_and_supplements_dict(self, plant): - pv_plant, supplements = design_plant(plant) + pv_plant, supplements = design_plant(plant.with_defaults_applied()) assert isinstance(pv_plant, PVPlant) assert isinstance(supplements, dict) @@ -119,8 +119,8 @@ def test_higher_capacity_produces_more_inverters(self, bern_2d_racks_inputs): def total_inverters(pv_plant): return sum(inv.inverter_count for t in pv_plant.transformers for inv in t.inverters) - small_plant, _ = design_plant(small) - large_plant, _ = design_plant(large) + small_plant, _ = design_plant(small.with_defaults_applied()) + large_plant, _ = design_plant(large.with_defaults_applied()) assert total_inverters(small_plant) < total_inverters(large_plant) @@ -139,7 +139,7 @@ def test_flush_mount_uses_plant_tilt(self): specs = generate_mounting_specs( mounting_type_id="Mounting Type 1", - plant=plant, + plant=plant.with_defaults_applied(), are_modules_landscape=False, rack_height=2.0, y_spacing=0.02, diff --git a/tests/test_pvsystem.py b/tests/test_pvsystem.py index 09dfb42..95efc19 100644 --- a/tests/test_pvsystem.py +++ b/tests/test_pvsystem.py @@ -1,4 +1,5 @@ import json +from dataclasses import replace from pathlib import Path import pytest @@ -214,60 +215,6 @@ def test_pvsystem_enum_reassignment_invalid_value_raises(self): with pytest.raises(ValueError, match="MountingType"): plant.mounting = None - def test_autofilled_defaults_follow_input_changes(self): - plant = PVSystem(latitude=45.0, longitude=10.0) - assert plant.mounting_height == 0.7 - assert plant.dc_ohmic_loss == 0.015 - - plant.mounting = "tracker" - plant.inverter_type = "string" - plant.bifacial = True - plant.transformer_stages = 0 - - assert plant.mounting_height == 1.5 - assert (plant.dc_ohmic_loss, plant.ac_ohmic_loss) == (0.007, 0.010) - assert plant.bifacial_shade_loss == 0.100 - assert plant.transformer_fixed_loss == 0 - - def test_explicit_values_are_not_overwritten(self): - plant = PVSystem(latitude=45.0, longitude=10.0, mounting_height=2.0, dc_ohmic_loss=0.02) - - plant.mounting = "Tracker" - plant.inverter_type = "String" - - assert plant.mounting_height == 2.0 - assert plant.dc_ohmic_loss == 0.02 - assert plant.ac_ohmic_loss == 0.010 # still auto-filled - - def test_assigned_value_stops_autofill_and_none_restores_it(self): - plant = PVSystem(latitude=45.0, longitude=10.0) - - plant.mounting_height = 3.0 - plant.mounting = "Tracker" - assert plant.mounting_height == 3.0 - - plant.mounting_height = None - assert plant.mounting_height == 1.5 - plant.mounting = "Fixed" - assert plant.mounting_height == 0.7 - - def test_make_copy_keeps_autofill_tracking(self): - plant = PVSystem(latitude=45.0, longitude=10.0, dc_ohmic_loss=0.02) - - copy = plant.make_copy() - copy.mounting = "Tracker" - copy.inverter_type = "String" - - assert copy.mounting_height == 1.5 - assert copy.dc_ohmic_loss == 0.02 - assert copy.ac_ohmic_loss == 0.010 - - def test_invalid_transformer_stages_on_reassignment_raises(self): - plant = PVSystem(latitude=45.0, longitude=10.0) - - with pytest.raises(ValueError, match="transformer_stages"): - plant.transformer_stages = 2 - def test_pvsystem_inverter_type_none_defaults_to_central(self): plant = PVSystem(latitude=45.0, longitude=10.0, inverter_type=None) @@ -284,6 +231,7 @@ def _plant(sample_data_dir): dc_capacity_MW=5.0, ac_capacity_MW=4.0, tilt=20.0, + print_design_summary=False, ) plant.pan_files = {"Module": data_dir / "CanadianSolar_CS6U-330M_APP.PAN"} plant.ond_files = {"Inverter": data_dir / "Sungrow_SG125HV_APP.OND"} @@ -758,14 +706,14 @@ def test_bifacial_tracker_construction_does_not_raise(self): def test_bifacial_fixed_defaults_are_correct(self): """Fixed-tilt bifacial defaults must match BIFACIAL_DICT[True]['Fixed'].""" - plant = PVSystem(bifacial=True, mounting="Fixed") + plant = PVSystem(bifacial=True, mounting="Fixed").with_defaults_applied() assert plant.bifacial_transmission == 0.05 assert plant.bifacial_shade_loss == 0.200 assert plant.bifacial_mismatch_loss == 0.01 def test_bifacial_tracker_defaults_are_correct(self): """Tracker bifacial defaults must match BIFACIAL_DICT[True]['Tracker'].""" - plant = PVSystem(bifacial=True, mounting="Tracker") + plant = PVSystem(bifacial=True, mounting="Tracker").with_defaults_applied() assert plant.bifacial_transmission == 0.05 assert plant.bifacial_shade_loss == 0.100 assert plant.bifacial_mismatch_loss == 0.005 @@ -778,19 +726,146 @@ def test_bifacial_explicit_override_is_preserved(self): bifacial_transmission=0.03, bifacial_shade_loss=0.15, bifacial_mismatch_loss=0.008, - ) + ).with_defaults_applied() assert plant.bifacial_transmission == 0.03 assert plant.bifacial_shade_loss == 0.15 assert plant.bifacial_mismatch_loss == 0.008 def test_non_bifacial_defaults_unchanged(self): """Regression: non-bifacial construction must still produce zero bifacial params.""" - plant = PVSystem(bifacial=False) + plant = PVSystem(bifacial=False).with_defaults_applied() assert plant.bifacial_transmission == 0.0 assert plant.bifacial_shade_loss == 0.0 assert plant.bifacial_mismatch_loss == 0.0 +class TestPVSystemDerivedDefaults: + def test_documented_flow_applies_defaults(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + plant.mounting = "Tracker" + plant.bifacial = True + plant.inverter_type = "String" + plant.transformer_stages = 0 + + derived_fields = ( + "mounting_height", + "dc_ohmic_loss", + "ac_ohmic_loss", + "bifacial_transmission", + "bifacial_shade_loss", + "bifacial_mismatch_loss", + "transformer_fixed_loss", + "transformer_variable_loss", + ) + assert all(getattr(plant, name) is None for name in derived_fields) + + resolved = plant.with_defaults_applied() + + assert resolved.mounting_height == 1.5 + assert ( + resolved.bifacial_transmission, + resolved.bifacial_shade_loss, + resolved.bifacial_mismatch_loss, + ) == (0.05, 0.100, 0.005) + assert (resolved.dc_ohmic_loss, resolved.ac_ohmic_loss) == (0.007, 0.010) + assert (resolved.transformer_fixed_loss, resolved.transformer_variable_loss) == (0, 0) + + def test_with_defaults_applied_does_not_mutate_original(self): + plant = PVSystem(latitude=45.0, longitude=10.0, mounting="Tracker", bifacial=True) + + resolved = plant.with_defaults_applied() + + assert resolved is not plant + assert plant.mounting_height is None + assert plant.bifacial_shade_loss is None + + def test_explicit_values_are_preserved_including_zero(self): + plant = PVSystem( + latitude=45.0, + longitude=10.0, + mounting_height=2.0, + bifacial_transmission=0.0, + ) + plant.mounting = "Tracker" + plant.bifacial = True + + resolved = plant.with_defaults_applied() + + assert resolved.mounting_height == 2.0 + assert resolved.bifacial_transmission == 0.0 + assert resolved.bifacial_shade_loss == 0.100 + + def test_defaults_follow_after_to_file_from_file(self, tmp_path): + plant = PVSystem(latitude=45.0, longitude=10.0) + file_path = tmp_path / "plant.json" + plant.to_file(file_path) + + loaded = PVSystem.from_file(file_path) + loaded.mounting = "Tracker" + + assert loaded.with_defaults_applied().mounting_height == 1.5 + + def test_defaults_follow_after_make_copy(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + copy = plant.make_copy() + copy.mounting = "Tracker" + + assert copy.with_defaults_applied().mounting_height == 1.5 + + def test_defaults_follow_after_dataclasses_replace(self): + plant = PVSystem(latitude=45.0, longitude=10.0) + + copy = replace(plant, name="Replacement") + copy.mounting = "Tracker" + + assert copy.with_defaults_applied().mounting_height == 1.5 + + def test_legacy_json_with_frozen_values_is_treated_as_explicit(self, tmp_path): + plant = PVSystem(latitude=45.0, longitude=10.0) + file_path = tmp_path / "plant.json" + plant.to_file(file_path) + plant_data = json.loads(file_path.read_text()) + plant_data["mounting_height"] = 0.7 + file_path.write_text(json.dumps(plant_data)) + + loaded = PVSystem.from_file(file_path) + loaded.mounting = "Tracker" + + assert loaded.with_defaults_applied().mounting_height == 0.7 + + def test_invalid_transformer_stages(self): + with pytest.raises(ValueError, match="transformer_stages"): + PVSystem(latitude=45.0, longitude=10.0, transformer_stages=3) + + plant = PVSystem(latitude=45.0, longitude=10.0) + plant.transformer_stages = 2 + with pytest.raises(ValueError, match="transformer_stages"): + plant.with_defaults_applied() + + def test_produce_payload_does_not_mutate_plant_inputs(self, sample_data_dir): + plant = TestPVSystemPayloadSnapshot._plant(sample_data_dir) + plant.gcr = 0.4 + first_payload = plant.produce_payload() + first_summary = plant.design_summary.copy() + second_payload = plant.produce_payload() + + assert plant.dc_capacity_MW == 5.0 + assert plant.ac_capacity_MW == 4.0 + assert plant.gcr == 0.4 + assert first_summary["target_dc_capacity_MW"] == 5.0 + assert plant.design_summary["target_dc_capacity_MW"] == 5.0 + assert first_summary == plant.design_summary + assert first_payload == second_payload + + def test_describe_prints_effective_defaults(self, capsys): + plant = PVSystem(latitude=45.0, longitude=10.0, mounting="Tracker") + + plant.describe(verbose=True) + + assert "Mounting Height: 1.5" in capsys.readouterr().out + + class TestPVSystemPersistence: """Tests for to_file() / from_file() round-trip (Phase 3 bug fix).""" From 2995573bb7dc329329284948f310e3f02fb14a8a Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Fri, 16 Oct 2026 13:25:31 -0700 Subject: [PATCH 06/12] Document and type PVSystem defaults Document case-insensitive enum inputs, build-time derived defaults, and output-only payload behavior. Widen the enum field annotations to include string inputs and record the compatibility changes in the changelog. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 26 ++++++- .../workflow-2-pvplant-builder.md | 9 +++ solarfarmer/models/pvsystem/pvsystem.py | 76 +++++++++++-------- 3 files changed, 78 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5686c25..03c688f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,30 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- `PVSystem` enum inputs (`mounting`, `module_orientation`, `inverter_type`) accept case-insensitive strings, + at construction and on assignment. +- `PVSystem.with_defaults_applied()` returns a copy with all unset derived inputs resolved. + +### Changed + +- Derived `PVSystem` inputs (`mounting_height`, ohmic losses, bifacial parameters, transformer losses) are now + resolved when the payload is built; they read as `None` until set explicitly. Use `describe()` or + `with_defaults_applied()` to see effective values. +- `PVSystem.payload` is now an output-only snapshot (JSON string) and is no longer read back; it is omitted + from `to_file()` output. +- Building a payload no longer overwrites the plant's `dc_capacity_MW`, `ac_capacity_MW` or `gcr`. + +### Fixed + +- Payloads built after changing `mounting`, `bifacial`, `inverter_type` or `transformer_stages` now use the + corresponding derived defaults. Previously they were silently ignored, e.g. bifacial losses stayed 0 + after `plant.bifacial = True`; results from workflows that assign these after construction will change. +- A stale cached payload could be sent after editing a plant (including via `make_copy()`). + ## [0.7.0] - 2026-10-07 ### Added @@ -31,7 +55,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Flush-mount `PVSystem` tilt handling now preserves `plant.tilt` in mounting specifications instead of forcing `0.0`, aligning flush-mount behaviour with - user-provided fixed-tilt inputs. + user-provided fixed-tilt inputs. ## [0.6.0] - 2026-09-24 diff --git a/docs/getting-started/workflow-2-pvplant-builder.md b/docs/getting-started/workflow-2-pvplant-builder.md index 0b5362f..8a7932f 100644 --- a/docs/getting-started/workflow-2-pvplant-builder.md +++ b/docs/getting-started/workflow-2-pvplant-builder.md @@ -124,6 +124,15 @@ plant.bifacial_shade_loss = 0.15 plant.bifacial_mismatch_loss = 0.01 ``` +### Derived defaults + +Unset mounting height, inverter and transformer losses, and bifacial parameters are derived when the +payload is built, using the final values of `mounting`, `bifacial`, `inverter_type` and +`transformer_stages`. You can assign these inputs in any order. Set an individual derived field to +override its default; explicit values including `0.0` are preserved. Read effective values with +`plant.with_defaults_applied()` or `plant.describe(verbose=True)`. Enum strings such as `"tracker"` +are accepted case-insensitively. + --- ## Step 3: Add Module and Inverter Files diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index 75d4fd7..42247ee 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -151,17 +151,21 @@ class PVSystem: Defines the symmetric rotation bounds: ``[-tracker_max_rotation_angle, +tracker_max_rotation_angle]``. azimuth : float Array azimuth in degrees (default 180, i.e., south-facing). - mounting: str + mounting : str Mounting configuration: 'Fixed' for fixed-tilt or 'Tracker' for single-axis trackers. - Available as ``solarfarmer.MountingType`` enum (e.g., ``sf.MountingType.FIXED``). + Strings are case-insensitive (e.g., ``"tracker"``) and are stored as + ``solarfarmer.MountingType`` enum values. + module_orientation : str + Module orientation: 'Portrait' or 'Landscape'. Strings are case-insensitive and are + stored as ``solarfarmer.OrientationType`` enum values. flush_mount : bool If True, indicates flush-mounted arrays (default is False). bifacial: bool If True, indicates bifacial modules (default is False). - inverter_type: str + inverter_type : str Inverter type: 'Central' or 'String' (default is 'Central'). - Affects the default DC and AC ohmic losses used, which can be overridden via - `dc_ohmic_loss` and `ac_ohmic_loss`. + Strings are case-insensitive and are stored as ``solarfarmer.InverterType`` enum values. + Determines the default DC and AC ohmic losses when building a payload. transformer_stages: int Number of transformer stages. 0 for ideal behaviour or 1 for one stage (default is 1). plant_unavailability : float @@ -182,19 +186,20 @@ class PVSystem: Other Effects and Settings -------------------------- - mounting_height: Optional[float] = None + mounting_height : Optional[float] Mounting height in meters. It refers to the minimum height from ground for fixed-tilt - and axis-tube height for single-axis trackers (default is 1.5 m). + and axis-tube height for single-axis trackers. ``None`` means derive from ``mounting`` + when building the payload; the default is 0.7 m for fixed mounting and 1.5 m for trackers. modules_across: Optional[int] = None Number of modules across in the mounting configuration (default is 1 for both trackers and fixed-tilt). string_length: Optional[int] = None Number of modules in series per string (default is calculated from the module and inverter provided). - dc_ohmic_loss: Optional[float] = None - DC ohmic loss (per unit) (default depends on inverter type). - ac_ohmic_loss: Optional[float] = None - AC ohmic loss (per unit) (default depends on inverter type). + dc_ohmic_loss : Optional[float] + DC ohmic loss (per unit). ``None`` means derive from ``inverter_type`` at build time. + ac_ohmic_loss : Optional[float] + AC ohmic loss (per unit). ``None`` means derive from ``inverter_type`` at build time. module_mismatch: Optional[float] = None Module mismatch loss (per unit) (default is 0.005). recalculate_modeling_correction_factor: bool @@ -216,16 +221,19 @@ class PVSystem: Monthly soiling loss (per unit), 12 values in [0,1]. If set with 1 value, it's expanded to 12. albedo: Optional[List[float]] = None Monthly albedo (per unit), 12 values in [0,1]. If set with 1 value, it's expanded to 12. - transformer_fixed_loss : Optional[float] = None - Transformer fixed loss (per unit). - transformer_variable_loss : Optional[float] = None - Transformer variable loss (per unit). - bifacial_transmission : Optional[float] = None - Bifacial transmission gain (per unit). Applicable only if `bifacial` is True. - bifacial_shade_loss : Optional[float] = None - Bifacial shade loss (per unit). Applicable only if `bifacial` is True. - bifacial_mismatch_loss : Optional[float] = None - Bifacial mismatch loss (per unit). Applicable only if `bifacial` is True. + transformer_fixed_loss : Optional[float] + Transformer fixed loss (per unit). ``None`` means derive from ``transformer_stages``. + transformer_variable_loss : Optional[float] + Transformer variable loss (per unit). ``None`` means derive from ``transformer_stages``. + bifacial_transmission : Optional[float] + Bifacial transmission gain (per unit). ``None`` means derive from ``bifacial`` and + ``mounting`` at build time. + bifacial_shade_loss : Optional[float] + Bifacial shade loss (per unit). ``None`` means derive from ``bifacial`` and + ``mounting`` at build time. + bifacial_mismatch_loss : Optional[float] + Bifacial mismatch loss (per unit). ``None`` means derive from ``bifacial`` and + ``mounting`` at build time. aux_loss_fixed_factor: Optional[float] = None Simple loss as a fraction of AC output (per unit). aux_loss_power: Optional[float] = None @@ -262,10 +270,14 @@ class PVSystem: - from_file(path) - make_copy() - payload_to_file(path) + - with_defaults_applied() Notes ----- - All setters accept `str` or `Path` for paths; stored internally as `Path`. + - Unset derived inputs remain ``None`` until used to build a payload. Use + ``with_defaults_applied()`` to inspect effective values; explicit values, including + ``0.0``, override the derived defaults. """ # Basic metadata @@ -296,21 +308,21 @@ class PVSystem: None # For trackers only; default is MAX_ROTATION_ANGLE (60°) ) azimuth: float = 180.0 - mounting: MountingType = MountingType.FIXED + mounting: MountingType | str = MountingType.FIXED flush_mount: bool = False bifacial: bool = False - inverter_type: InverterType | None = InverterType.CENTRAL + inverter_type: InverterType | str | None = InverterType.CENTRAL transformer_stages: int = 1 plant_unavailability: float = 0.0 grid_unavailability: float = 0.0 # Effects and settings - mounting_height: float | None = None # Changed from: mounting_height: float + mounting_height: float | None = None # None = derive from mounting at build time modules_across: int | None = None - module_orientation: OrientationType = OrientationType.PORTRAIT + module_orientation: OrientationType | str = OrientationType.PORTRAIT string_length: int | None = None # Estimated from module and inverter provided - dc_ohmic_loss: float | None = None # Default depends on inverter type - ac_ohmic_loss: float | None = None # Default depends on inverter type + dc_ohmic_loss: float | None = None # None = derive from inverter_type at build time + ac_ohmic_loss: float | None = None # None = derive from inverter_type at build time module_mismatch: float | None = MODULE_MISMATCH_FACTOR recalculate_modeling_correction_factor: bool = True module_quality_factor: float | None = 0.0 @@ -318,11 +330,11 @@ class PVSystem: module_iam_model_override: str | None = None constant_heat_coefficient: float | None = CONSTANT_HEAT_COEFFICIENT convective_heat_coefficient: float | None = CONVECTIVE_HEAT_COEFFICIENT - transformer_fixed_loss: float | None = None # Default depends on transformer stages - transformer_variable_loss: float | None = None # Default depends on transformer stages - bifacial_transmission: float | None = None # Default depends on bifacial flag - bifacial_shade_loss: float | None = None # Default depends on bifacial flag - bifacial_mismatch_loss: float | None = None # Default depends on bifacial flag + transformer_fixed_loss: float | None = None # None = derive from transformer_stages + transformer_variable_loss: float | None = None # None = derive from transformer_stages + bifacial_transmission: float | None = None # None = derive from bifacial and mounting + bifacial_shade_loss: float | None = None # None = derive from bifacial and mounting + bifacial_mismatch_loss: float | None = None # None = derive from bifacial and mounting aux_loss_fixed_factor: float | None = None aux_loss_power: float | None = None aux_loss_apply_at_night: bool | None = None From a343acac9d2ea1fd74f1be8a134fe457ce53608e Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Fri, 16 Oct 2026 13:59:31 -0700 Subject: [PATCH 07/12] Note shared containers in with_defaults_applied Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- solarfarmer/models/pvsystem/pvsystem.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index 42247ee..be60a20 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -729,6 +729,8 @@ def with_defaults_applied(self) -> PVSystem: >>> plant.mounting_height is None True """ + # Shallow copy: containers such as _pan_files and _albedo are shared with self, + # so the build path must only read them, never mutate them in place. resolved = replace(self, results=None, payload=None, design_summary=None) for name, default in _default_derived_values(resolved).items(): if getattr(resolved, name) is None: From b61c91300fa429b4d1f9cea865802055dbb3220e Mon Sep 17 00:00:00 2001 From: Javier Lopez Date: Sat, 17 Oct 2026 00:29:36 +0200 Subject: [PATCH 08/12] Fix inverter efficiency formatting in set_inverter_from_rcl error (#47) * Fix inverter efficiency formatting in set_inverter_from_rcl error * Add regression test for set_inverter_from_rcl efficiency formatting Cover the "Narrow your search" ValueError raised when multiple inverters match, asserting efficMax is listed as a percentage (eff=98.6%) rather than scaled by 100. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add changelog entry for set_inverter_from_rcl efficiency fix Also strips a trailing space on an existing 0.7.0 entry, as enforced by the trailing-whitespace pre-commit hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Ian Tse Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 9 ++++++++- solarfarmer/models/pvsystem/pvsystem.py | 2 +- tests/test_rcl.py | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5686c25..eda7a81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- `PVSystem.set_inverter_from_rcl()` no longer multiplies the inverter efficiency by 100 in the + "Narrow your search" message (e.g. `eff=9860.0%` is now `eff=98.6%`). + ## [0.7.0] - 2026-10-07 ### Added @@ -31,7 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Flush-mount `PVSystem` tilt handling now preserves `plant.tilt` in mounting specifications instead of forcing `0.0`, aligning flush-mount behaviour with - user-provided fixed-tilt inputs. + user-provided fixed-tilt inputs. ## [0.6.0] - 2026-09-24 diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index 2247e5c..2d62235 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -1294,7 +1294,7 @@ def set_inverter_from_rcl( if pnom is not None: extras.append(f"{pnom:g}kW") if eff is not None: - extras.append(f"eff={eff * 100:.1f}%") + extras.append(f"eff={eff:.1f}%") if mppt is not None: extras.append(f"{mppt} MPPT") if extras: diff --git a/tests/test_rcl.py b/tests/test_rcl.py index 035ef2f..eec54c2 100644 --- a/tests/test_rcl.py +++ b/tests/test_rcl.py @@ -6,6 +6,7 @@ import pytest import solarfarmer.rcl as rcl +from solarfarmer import PVSystem from solarfarmer.rcl import ( RCLModuleCatalogResponse, RCLRateLimitInfo, @@ -418,6 +419,24 @@ def test_raises_when_headers_missing(self, MockClient): rcl.get_rate_limit_status() +class TestSetInverterFromRclUnit: + """Unit tests for PVSystem.set_inverter_from_rcl with mocked catalog search.""" + + @patch("solarfarmer.rcl.list_inverters") + def test_multiple_matches_list_efficiency_as_percentage(self, mock_list): + # efficMax is already a percentage in the catalog, so it must not be scaled by 100 + mock_list.return_value = { + "items": [ + {"manufacturer": "SMA", "model": "A", "pNomConv": 4000.0, "efficMax": 98.6}, + {"manufacturer": "SMA", "model": "B", "pNomConv": 3000.0, "efficMax": 98.2}, + ], + "total": 2, + } + with pytest.raises(ValueError, match=r"SMA - A \(4000kW, eff=98\.6%\)") as exc_info: + PVSystem().set_inverter_from_rcl(manufacturer="SMA") + assert "eff=98.2%" in str(exc_info.value) + + # --------------------------------------------------------------------------- # Integration tests — require SF_API_KEY, auto-skip when absent # --------------------------------------------------------------------------- From 80099d049e09231dfc98afd14793beb7abeddd3a Mon Sep 17 00:00:00 2001 From: Javier Lopez Lorente Date: Mon, 19 Oct 2026 14:40:41 +0200 Subject: [PATCH 09/12] Rebase to main and reapply fix. --- solarfarmer/models/pvsystem/pvsystem.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index 92b56ac..18c4280 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -1035,10 +1035,11 @@ def payload_to_file(self, file_path: PathLike) -> None: path = Path(file_path) path.parent.mkdir(parents=True, exist_ok=True) # Ensure parent directories exist - payload = self.produce_payload() + # produce_payload returns compact JSON; re-serialize so the file is indented + payload = json.loads(self.produce_payload()) with path.open("w", encoding="utf-8") as f: - f.write(payload) + json.dump(payload, f, indent=4) _logger.debug("PVSystem payload saved to %s", path) return None From ffc7e09e15023e7222dadc86b84b73bf0c76217d Mon Sep 17 00:00:00 2001 From: Javier Lopez Lorente Date: Mon, 19 Oct 2026 14:40:52 +0200 Subject: [PATCH 10/12] Add unit test. --- tests/test_pvsystem.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test_pvsystem.py b/tests/test_pvsystem.py index 95efc19..a92f7bd 100644 --- a/tests/test_pvsystem.py +++ b/tests/test_pvsystem.py @@ -266,6 +266,17 @@ def test_make_copy_resets_payload(self, sample_data_dir): assert plant.make_copy().payload is None + def test_payload_to_file_is_indented(self, sample_data_dir, tmp_path): + plant = self._plant(sample_data_dir) + file_path = tmp_path / "payload.json" + + plant.payload_to_file(file_path) + + text = file_path.read_text(encoding="utf-8") + assert text.count("\n") > 1 + assert '\n "' in text + assert json.loads(text) == json.loads(plant.payload) + def test_to_file_omits_payload(self, sample_data_dir, tmp_path): plant = self._plant(sample_data_dir) plant.produce_payload() From 9c119aafa037c50dbb11ef27ca105d57d5bacf9b Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 19 Oct 2026 09:47:04 -0700 Subject: [PATCH 11/12] Preserve non-ASCII characters in PVSystem.payload_to_file() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- solarfarmer/models/pvsystem/pvsystem.py | 2 +- tests/test_pvsystem.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index 18c4280..7b9826e 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -1039,7 +1039,7 @@ def payload_to_file(self, file_path: PathLike) -> None: payload = json.loads(self.produce_payload()) with path.open("w", encoding="utf-8") as f: - json.dump(payload, f, indent=4) + json.dump(payload, f, indent=4, ensure_ascii=False) _logger.debug("PVSystem payload saved to %s", path) return None diff --git a/tests/test_pvsystem.py b/tests/test_pvsystem.py index a92f7bd..bde7af2 100644 --- a/tests/test_pvsystem.py +++ b/tests/test_pvsystem.py @@ -277,6 +277,19 @@ def test_payload_to_file_is_indented(self, sample_data_dir, tmp_path): assert '\n "' in text assert json.loads(text) == json.loads(plant.payload) + def test_payload_to_file_keeps_non_ascii_characters(self, sample_data_dir, tmp_path): + plant = self._plant(sample_data_dir) + pan_path = tmp_path / "Módulo.PAN" + pan_path.write_bytes(plant.pan_files["Module"].read_bytes()) + plant.pan_files = {"Module": pan_path} + file_path = tmp_path / "payload.json" + + plant.payload_to_file(file_path) + + text = file_path.read_text(encoding="utf-8") + assert "Módulo" in text + assert "\\u00f3" not in text + def test_to_file_omits_payload(self, sample_data_dir, tmp_path): plant = self._plant(sample_data_dir) plant.produce_payload() From 9154d2defd91bd750fb90cdd7d51d01b470a4752 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 19 Oct 2026 09:48:32 -0700 Subject: [PATCH 12/12] Add changelog entry for payload_to_file formatting fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10a0f02..7e1328e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A stale cached payload could be sent after editing a plant (including via `make_copy()`). - `PVSystem.set_inverter_from_rcl()` no longer multiplies the inverter efficiency by 100 in the "Narrow your search" message (e.g. `eff=9860.0%` is now `eff=98.6%`). +- `PVSystem.payload_to_file()` now writes indented JSON instead of a single compact line, and keeps + non-ASCII characters (e.g. in module or inverter file names) instead of escaping them as `\uXXXX`. ## [0.7.0] - 2026-10-07