From 93b5de819406c63fef9b02ed4ca67bc2ad3f4d65 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 10:14:36 -0700 Subject: [PATCH 01/12] Surface API error details by correcting problem_details_json type and removing silent exception swallowing --- solarfarmer/api.py | 4 ++-- solarfarmer/endpoint_modelchains.py | 27 +++++++++++----------- tests/test_api.py | 36 +++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/solarfarmer/api.py b/solarfarmer/api.py index 4f4754b..2189c3e 100644 --- a/solarfarmer/api.py +++ b/solarfarmer/api.py @@ -25,7 +25,7 @@ class Response: success: bool method: str exception: str | None = None - problem_details_json: str | None = None + problem_details_json: dict | None = None def __repr__(self) -> str: return f"status code={self.code}, url={self.url}, method={self.method}" @@ -237,7 +237,7 @@ def _make_request( try: problem_details_json = response.json() except Exception: - problem_details_json = "" + problem_details_json = None return self.response_class( code=response.status_code, diff --git a/solarfarmer/endpoint_modelchains.py b/solarfarmer/endpoint_modelchains.py index 4d31dc7..299f274 100644 --- a/solarfarmer/endpoint_modelchains.py +++ b/solarfarmer/endpoint_modelchains.py @@ -225,19 +225,20 @@ def _log_api_failure(response: Response, elapsed_time: float) -> None: elapsed_time, ) _logger.error("Failure message: %s", response.exception) - try: - json_response = response.problem_details_json - if json_response is not None and "title" in json_response: - _logger.error("Title: %s", json_response["title"]) - if json_response.get("errors") is not None: - _logger.error("Errors:") - for _errorKey, errors in json_response["errors"].items(): - for error in errors: - _logger.error(" - %s", error) - if json_response.get("detail") is not None: - _logger.error("Detail: %s", json_response["detail"]) - except Exception: - pass + json_response = response.problem_details_json + if json_response is None: + return + if "title" in json_response: + _logger.error("Title: %s", json_response["title"]) + errors = json_response.get("errors") + if errors is not None: + _logger.error("Errors:") + for _errorKey, error_list in errors.items(): + for error in error_list: + _logger.error(" - %s", error) + detail = json_response.get("detail") + if detail is not None: + _logger.error("Detail: %s", detail) def run_energy_calculation( diff --git a/tests/test_api.py b/tests/test_api.py index 788aa44..4899fed 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -368,6 +368,20 @@ def test_timeout_error_returns_failure(self, client): assert resp.code == 0 assert "Network error" in resp.exception + def test_400_non_json_body_sets_problem_details_to_none(self, client): + error_response = MagicMock() + error_response.ok = False + error_response.status_code = 400 + error_response.url = f"{BASE_URL}/About" + error_response.text = "Bad Request" + error_response.json.side_effect = ValueError("No JSON") + + with patch("solarfarmer.api.requests.request", return_value=error_response): + resp = client.get({"api_key": FAKE_KEY}) + + assert resp.success is False + assert resp.problem_details_json is None + class TestMakeRequestPost: def test_200_post_returns_success_response(self, monkeypatch, mock_http_response): @@ -410,3 +424,25 @@ def test_400_post_with_provider_message(self, monkeypatch): assert resp.success is False assert "Invalid panel count" in resp.exception + + def test_400_non_json_body_sets_problem_details_to_none(self, monkeypatch): + monkeypatch.setattr(api_module, "API_TOKEN", FAKE_KEY) + c = Client( + base_url=BASE_URL, + endpoint="ModelChain", + response_type=Response, + timeout=MODELCHAIN_TIMEOUT, + ) + + error_response = MagicMock() + error_response.ok = False + error_response.status_code = 400 + error_response.url = f"{BASE_URL}/ModelChain" + error_response.text = "Bad Request" + error_response.json.side_effect = ValueError("No JSON") + + with patch("solarfarmer.api.requests.post", return_value=error_response): + resp = c.post({"api_key": FAKE_KEY}, request_content="{}", files=None) + + assert resp.success is False + assert resp.problem_details_json is None From 2f4392b7196cbe8862620b36577e912abcd463d8 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 10:47:36 -0700 Subject: [PATCH 02/12] Raise SolarFarmerAPIError on API failure instead of returning None --- solarfarmer/__init__.py | 2 + solarfarmer/api.py | 43 +++++++++++++++ solarfarmer/endpoint_modelchains.py | 25 +++++++-- tests/test_endpoint_modelchain.py | 84 ++++++++++++++++++++++------- 4 files changed, 132 insertions(+), 22 deletions(-) diff --git a/solarfarmer/__init__.py b/solarfarmer/__init__.py index 2e30a89..c88680b 100644 --- a/solarfarmer/__init__.py +++ b/solarfarmer/__init__.py @@ -24,6 +24,7 @@ from .endpoint_modelchains import run_energy_calculation from .endpoint_service import service from .endpoint_terminate_async import terminate_calculation +from .api import SolarFarmerAPIError from .logging import configure_logging from .models import ( AuxiliaryLosses, @@ -104,6 +105,7 @@ "PVSystem", "run_energy_calculation", "service", + "SolarFarmerAPIError", "terminate_calculation", "TrackerSystem", "Transformer", diff --git a/solarfarmer/api.py b/solarfarmer/api.py index 2189c3e..586cb96 100644 --- a/solarfarmer/api.py +++ b/solarfarmer/api.py @@ -31,6 +31,49 @@ def __repr__(self) -> str: return f"status code={self.code}, url={self.url}, method={self.method}" +class SolarFarmerAPIError(Exception): + """Raised when the SolarFarmer API returns a non-2xx response. + + Attributes + ---------- + status_code : int + HTTP status code returned by the API. + message : str + Human-readable error message extracted from the response. + problem_details : dict or None + Full ProblemDetails JSON body from the API, if available. + May contain ``title``, ``detail``, and ``errors`` fields. + + Examples + -------- + >>> import solarfarmer as sf + >>> try: + ... result = sf.run_energy_calculation(inputs_folder_path="my_inputs/") + ... except sf.SolarFarmerAPIError as e: + ... logger.error("API error %s: %s", e.status_code, e) + ... raise + """ + + def __init__( + self, + status_code: int, + message: str, + problem_details: dict | None = None, + ) -> None: + super().__init__(message) + self.status_code = status_code + self.message = message + self.problem_details = problem_details + + def __str__(self) -> str: + base = f"HTTP {self.status_code}: {self.message}" + if self.problem_details: + detail = self.problem_details.get("detail") + if detail: + base += f" — {detail}" + return base + + class Client: """Handles all API requests for the different endpoints.""" diff --git a/solarfarmer/endpoint_modelchains.py b/solarfarmer/endpoint_modelchains.py index 299f274..8df5597 100644 --- a/solarfarmer/endpoint_modelchains.py +++ b/solarfarmer/endpoint_modelchains.py @@ -3,7 +3,7 @@ from datetime import datetime from typing import IO, Any -from .api import Client, Response, build_api_url +from .api import Client, Response, SolarFarmerAPIError, build_api_url from .config import ( MODELCHAIN_ASYNC_ENDPOINT_URL, MODELCHAIN_ASYNC_TIMEOUT_CONNECTION, @@ -201,7 +201,14 @@ def _handle_successful_response( runtime_status, elapsed_time, ) - return None + # "Terminated" means the user explicitly cancelled via terminate_calculation() — not an error. + # All other non-Completed statuses (Failed, Canceled, Unknown) are unexpected failures. + if runtime_status == "Terminated": + return None + message = f"Async calculation ended with status '{runtime_status}'" + if output_message: + message += f": {output_message}" + raise SolarFarmerAPIError(status_code=200, message=message) def _log_api_failure(response: Response, elapsed_time: float) -> None: @@ -343,7 +350,13 @@ def run_energy_calculation( ------- CalculationResults or None An instance of CalculationResults with the API results for the project, - or None if the calculation failed or was terminated + or None if the calculation was terminated or cancelled + + Raises + ------ + SolarFarmerAPIError + If the API returns a non-2xx response. The exception carries + ``status_code``, ``message``, and the full ``problem_details`` body. """ # Fold explicit API params into kwargs for the downstream request functions @@ -399,7 +412,11 @@ def run_energy_calculation( ) else: _log_api_failure(response, elapsed_time) - return None + raise SolarFarmerAPIError( + status_code=response.code, + message=response.exception or "API request failed", + problem_details=response.problem_details_json, + ) def modelchain_call( diff --git a/tests/test_endpoint_modelchain.py b/tests/test_endpoint_modelchain.py index a91ad29..a6a773c 100644 --- a/tests/test_endpoint_modelchain.py +++ b/tests/test_endpoint_modelchain.py @@ -6,6 +6,7 @@ import solarfarmer from solarfarmer.api import Response +from solarfarmer.api import SolarFarmerAPIError from solarfarmer.endpoint_modelchains import ( _handle_successful_response, _log_api_failure, @@ -179,12 +180,11 @@ def test_terminated_async_returns_none(self): [ ("Failed", "Out of memory"), ("Canceled", None), - ("Terminated", "Manual stop"), ("Unknown", None), ], - ids=["failed_with_output", "canceled_no_output", "terminated_via_post", "unknown"], + ids=["failed_with_output", "canceled_no_output", "unknown"], ) - def test_non_completed_status_returns_none_and_logs(self, status, output, caplog): + def test_non_completed_status_raises_and_logs(self, status, output, caplog): data: dict = {"runtimeStatus": status} if output is not None: data["output"] = output @@ -193,14 +193,33 @@ def test_non_completed_status_returns_none_and_logs(self, status, output, caplog url="https://api.example.com/modelchainasync", data=data, success=True, - method="POST", # POST bypasses the early-exit Terminated guard + method="POST", + ) + with caplog.at_level(logging.ERROR): + with pytest.raises(SolarFarmerAPIError) as exc_info: + _handle_successful_response( + response, 3.0, "proj1", None, None, None, False, False + ) + assert f"Runtime status = {status}" in caplog.text + assert status in str(exc_info.value) + if output: + assert output in str(exc_info.value) + + def test_terminated_via_post_returns_none_and_logs(self, caplog): + """Terminated via POST (e.g. polled after user cancel) returns None, does not raise.""" + response = Response( + code=200, + url="https://api.example.com/modelchainasync", + data={"runtimeStatus": "Terminated", "output": "Manual stop"}, + success=True, + method="POST", ) with caplog.at_level(logging.ERROR): result = _handle_successful_response( response, 3.0, "proj1", None, None, None, False, False ) assert result is None - assert f"Runtime status = {status}" in caplog.text + assert "Runtime status = Terminated" in caplog.text def test_terminated_guard_requires_async_url(self): """GET + Terminated on a sync URL must NOT trigger the early-exit guard.""" @@ -356,9 +375,7 @@ def test_async_endpoint_selection( @patch("solarfarmer.endpoint_modelchains.modelchain_call") @patch("solarfarmer.endpoint_modelchains.check_for_3d_files", return_value=False) @patch("solarfarmer.endpoint_modelchains._resolve_request_payload") - def test_failed_response_logs_and_returns_none( - self, mock_resolve_payload, _mock_check_for_3d, mock_modelchain_call, mock_log_failure - ): + def test_failed_response_logs_and_raises(self, mock_resolve_payload, _mock_check_for_3d, mock_modelchain_call, mock_log_failure): mock_resolve_payload.return_value = ('{"pvPlant":{}}', []) mock_modelchain_call.return_value = Response( code=500, @@ -369,12 +386,42 @@ def test_failed_response_logs_and_returns_none( exception="Server error", ) - result = run_energy_calculation( - inputs_folder_path="/tmp/input-folder", project_id="project-1" - ) + with pytest.raises(SolarFarmerAPIError) as exc_info: + run_energy_calculation( + inputs_folder_path="/tmp/input-folder", project_id="project-1" + ) - assert result is None mock_log_failure.assert_called_once() + assert exc_info.value.status_code == 500 + assert exc_info.value.message == "Server error" + + @patch("solarfarmer.endpoint_modelchains._log_api_failure") + @patch("solarfarmer.endpoint_modelchains.modelchain_call") + @patch("solarfarmer.endpoint_modelchains.check_for_3d_files", return_value=False) + @patch("solarfarmer.endpoint_modelchains._resolve_request_payload") + def test_failed_response_carries_problem_details( + self, mock_resolve_payload, _mock_check_for_3d, mock_modelchain_call, _mock_log_failure + ): + mock_resolve_payload.return_value = ('{"pvPlant":{}}', []) + problem_details = {"title": "Validation failed", "detail": "rack height too small"} + mock_modelchain_call.return_value = Response( + code=400, + url="https://api.example.com/modelchain", + data=None, + success=False, + method="POST", + exception="Bad Request", + problem_details_json=problem_details, + ) + + with pytest.raises(SolarFarmerAPIError) as exc_info: + run_energy_calculation( + inputs_folder_path="/tmp/input-folder", project_id="project-1" + ) + + assert exc_info.value.status_code == 400 + assert exc_info.value.problem_details == problem_details + assert "rack height too small" in str(exc_info.value) @patch("solarfarmer.endpoint_modelchains.modelchain_call", side_effect=RuntimeError("boom")) @patch("solarfarmer.endpoint_modelchains.check_for_3d_files", return_value=False) @@ -405,12 +452,13 @@ def test_none_params_not_folded_into_kwargs( method="POST", exception="error", ) - run_energy_calculation( - inputs_folder_path="/tmp/input-folder", - api_key=None, - time_out=None, - async_poll_time=None, - ) + with pytest.raises(SolarFarmerAPIError): + run_energy_calculation( + inputs_folder_path="/tmp/input-folder", + api_key=None, + time_out=None, + async_poll_time=None, + ) kwargs_passed = mock_modelchain_call.call_args.kwargs assert "api_key" not in kwargs_passed assert "time_out" not in kwargs_passed From de74d2a8657776e125caebce9ec689a0d315f1cc Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 10:50:00 -0700 Subject: [PATCH 03/12] Correct PVSystem.run_energy_calculation return type and return the result --- solarfarmer/models/pvsystem/pvsystem.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/solarfarmer/models/pvsystem/pvsystem.py b/solarfarmer/models/pvsystem/pvsystem.py index aba179d..35ae995 100644 --- a/solarfarmer/models/pvsystem/pvsystem.py +++ b/solarfarmer/models/pvsystem/pvsystem.py @@ -941,7 +941,7 @@ def run_energy_calculation( force_async_call=False, api_version="latest", **kwargs, - ) -> None: + ) -> CalculationResults | None: """ Runs the SolarFArmer API calculation for the defined PV plant. @@ -975,8 +975,14 @@ def run_energy_calculation( Returns ------- - CalculationResults: - An instance of CalculationResults with the 2D API results for the PV plant. + CalculationResults or None + The calculation results on success, or None if the async calculation + was terminated by the user. Results are also stored in ``self.results``. + + Raises + ------ + SolarFarmerAPIError + If the API returns a non-2xx response or the async calculation fails. """ from ...endpoint_modelchains import run_energy_calculation @@ -1005,7 +1011,7 @@ def run_energy_calculation( # Store API results self.results = results - return None + return self.results def construct_plant(pvplant: PVSystem) -> str: From 591b56d9ef39f1fb059719964f191c094539ca91 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 10:54:12 -0700 Subject: [PATCH 04/12] =?UTF-8?q?Correct=20grid=5Fconnection=5Flimit=20uni?= =?UTF-8?q?ts=20in=20PVPlant=20docstring=20(MW=20=E2=86=92=20W)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- solarfarmer/models/pv_plant.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solarfarmer/models/pv_plant.py b/solarfarmer/models/pv_plant.py index 5722e25..7a1b10e 100644 --- a/solarfarmer/models/pv_plant.py +++ b/solarfarmer/models/pv_plant.py @@ -21,7 +21,8 @@ class PVPlant(SolarFarmerBaseModel): mounting_type_specifications : dict[str, MountingTypeSpecification] Mounting type specs keyed by ID grid_connection_limit : float or None - Maximum power that can be exported from the transformers in MW + Maximum power that can be exported from the transformers, in Watts (W). + Example: 17.6 MW → ``grid_connection_limit=17600000`` tracker_systems : dict[str, TrackerSystem] or None Tracker system specs keyed by ID. Required when layouts use trackers transformer_specifications : dict[str, TransformerSpecification] or None From f1e3f43e55f35c5dc2e610260038b1d39cc7821b Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 15:20:10 -0700 Subject: [PATCH 05/12] Added client-side validators and document spec ID conventions --- solarfarmer/endpoint_modelchains.py | 64 +++++++++++++++++++++++++++++ solarfarmer/models/inverter.py | 5 ++- solarfarmer/models/layout.py | 5 ++- solarfarmer/models/pv_plant.py | 21 +++++++++- solarfarmer/models/transformer.py | 5 ++- 5 files changed, 96 insertions(+), 4 deletions(-) diff --git a/solarfarmer/endpoint_modelchains.py b/solarfarmer/endpoint_modelchains.py index 8df5597..e96a55c 100644 --- a/solarfarmer/endpoint_modelchains.py +++ b/solarfarmer/endpoint_modelchains.py @@ -28,6 +28,69 @@ _logger = get_logger("endpoint.modelchains") +def _validate_spec_ids_match_files( + request_content: str, + files: list[tuple[str, IO[bytes]]], +) -> None: + """ + Validate that module and inverter spec IDs in the payload match uploaded file stems. + + The SolarFarmer API resolves ``moduleSpecificationID`` and ``inverterSpecID`` + by matching them against the filename stems of uploaded PAN and OND files + (i.e. filename without extension). A mismatch causes a KeyNotFoundException + on the server. This check catches the error client-side before the HTTP call. + + Parameters + ---------- + request_content : str + JSON-serialized API payload. + files : list of tuple[str, IO[bytes]] + Multipart upload files as ``(field_name, file_handle)`` pairs. + + Raises + ------ + ValueError + If a spec ID in the payload has no matching uploaded file. + """ + import json + + pan_stems = { + pathlib.Path(f.name).stem + for field, f in files + if field == "panFiles" and hasattr(f, "name") + } + ond_stems = { + pathlib.Path(f.name).stem + for field, f in files + if field == "ondFiles" and hasattr(f, "name") + } + + # Nothing to validate if no PAN/OND files were uploaded (e.g. inline payload) + if not pan_stems and not ond_stems: + return + + payload = json.loads(request_content) + pv_plant = payload.get("pvPlant", {}) + + for transformer in pv_plant.get("transformers", []): + for inverter in transformer.get("inverters", []): + if ond_stems: + inv_id = inverter.get("inverterSpecID") + if inv_id and inv_id not in ond_stems: + raise ValueError( + f"Inverter references spec ID '{inv_id}' but no matching OND file " + f"was uploaded. Available stems: {sorted(ond_stems)}" + ) + for layout in inverter.get("layouts") or []: + if pan_stems: + mod_id = layout.get("moduleSpecificationID") + if mod_id and mod_id not in pan_stems: + raise ValueError( + f"Layout references module spec '{mod_id}' but no matching PAN file " + f"was uploaded. Available stems: {sorted(pan_stems)}" + ) + + def _resolve_request_payload( inputs_folder_path: str | pathlib.Path | None, energy_calculation_inputs_file_path: str | None, @@ -381,6 +444,7 @@ def run_energy_calculation( ) # 2. Dispatch to the appropriate endpoint + _validate_spec_ids_match_files(request_content, files) are_files_3d = check_for_3d_files(request_content) start_time = time.time() if force_async_call or are_files_3d: diff --git a/solarfarmer/models/inverter.py b/solarfarmer/models/inverter.py index 91d55e0..3ba5c60 100644 --- a/solarfarmer/models/inverter.py +++ b/solarfarmer/models/inverter.py @@ -14,7 +14,10 @@ class Inverter(SolarFarmerBaseModel): Attributes ---------- inverter_spec_id : str - Reference to an inverter specification (OND file key) + Reference to an inverter specification. Must exactly match the + filename stem of the uploaded OND file (filename without the + ``.OND`` extension). Example: ``"Sungrow_SG125HV_APP"`` for a + file named ``Sungrow_SG125HV_APP.OND``. inverter_count : int Number of identical inverters, >= 1 layouts : list[Layout] or None diff --git a/solarfarmer/models/layout.py b/solarfarmer/models/layout.py index de043e0..fd63d24 100644 --- a/solarfarmer/models/layout.py +++ b/solarfarmer/models/layout.py @@ -14,7 +14,10 @@ class Layout(SolarFarmerBaseModel): layout_count : int Number of identical copies of this layout, >= 1 module_specification_id : str - Reference to a module specification (PAN file key) + Reference to a module specification. Must exactly match the + filename stem of the uploaded PAN file (filename without the + ``.PAN`` extension). Example: ``"Trina_TSM-DEG19C.20-550_APP"`` + for a file named ``Trina_TSM-DEG19C.20-550_APP.PAN``. mounting_type_id : str Reference to a mounting type specification is_trackers : bool diff --git a/solarfarmer/models/pv_plant.py b/solarfarmer/models/pv_plant.py index 7a1b10e..4e8d328 100644 --- a/solarfarmer/models/pv_plant.py +++ b/solarfarmer/models/pv_plant.py @@ -1,5 +1,7 @@ from __future__ import annotations +from pydantic import model_validator + from ._base import SolarFarmerBaseModel from .auxiliary_losses import AuxiliaryLosses from .mounting_type_specification import MountingTypeSpecification @@ -26,7 +28,9 @@ class PVPlant(SolarFarmerBaseModel): tracker_systems : dict[str, TrackerSystem] or None Tracker system specs keyed by ID. Required when layouts use trackers transformer_specifications : dict[str, TransformerSpecification] or None - Transformer specs keyed by ID + Transformer specs keyed by ID. Required by the API when the AC + model is enabled (the default). Omitting this field causes an + HTTP 400 error. The key must match ``Transformer.transformer_spec_id``. auxiliary_losses : AuxiliaryLosses or None Plant-level auxiliary losses """ @@ -37,3 +41,18 @@ class PVPlant(SolarFarmerBaseModel): tracker_systems: dict[str, TrackerSystem] | None = None transformer_specifications: dict[str, TransformerSpecification] | None = None auxiliary_losses: AuxiliaryLosses | None = None + + @model_validator(mode="after") + def _check_transformer_spec_references(self) -> "PVPlant": + """Validate that referenced transformer spec IDs exist in transformer_specifications.""" + for i, transformer in enumerate(self.transformers): + spec_id = transformer.transformer_spec_id + if spec_id is None: + continue + if self.transformer_specifications is None or spec_id not in self.transformer_specifications: + available = sorted(self.transformer_specifications or {}) + raise ValueError( + f"Transformer {i} references spec ID '{spec_id}' which is " + f"not in transformer_specifications. Available: {available}" + ) + return self diff --git a/solarfarmer/models/transformer.py b/solarfarmer/models/transformer.py index 5073c35..cfa5311 100644 --- a/solarfarmer/models/transformer.py +++ b/solarfarmer/models/transformer.py @@ -18,7 +18,10 @@ class Transformer(SolarFarmerBaseModel): name : str or None Optional descriptive name transformer_spec_id : str or None - Reference to a transformer specification + Reference to a transformer specification. Must match a key in + ``PVPlant.transformer_specifications``. Required by the API when + the AC model is enabled (``EnergyCalculationOptions.include_inverter_model=True``, + which is the default). Omitting it causes a runtime error. """ transformer_count: int = Field(1, ge=1) From e0a99b5a187c341658c63681e5fc249591d48fdb Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 15:30:10 -0700 Subject: [PATCH 06/12] Fix spacing params (pitch not gap) and document bifacial/rack height constraints --- .../models/mounting_type_specification.py | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/solarfarmer/models/mounting_type_specification.py b/solarfarmer/models/mounting_type_specification.py index 47ac233..7ebd242 100644 --- a/solarfarmer/models/mounting_type_specification.py +++ b/solarfarmer/models/mounting_type_specification.py @@ -15,9 +15,22 @@ class MountingTypeSpecification(SolarFarmerBaseModel): modules_are_landscape : bool Whether modules are mounted in landscape orientation rack_height : float - Height of the rack structure in metres, range [0, 100] + Height of the rack structure in meters, range [0, 100]. Must satisfy:: + + rack_height >= frame_bottom_width + + (module_side × number_of_modules_high) + + ((y_spacing_between_modules - module_side) × (number_of_modules_high - 1)) + + frame_top_width + + where ``module_side`` is the module length (if portrait) or width (if landscape) + as defined in the PAN file. Note: ``y_spacing_between_modules`` is the edge-to-edge + pitch (not gap), so ``(y_spacing_between_modules - module_side)`` gives the actual + gap. Violating this constraint causes a validation error. y_spacing_between_modules : float - Vertical gap between modules in metres, range [0, 5] + Edge-to-edge distance from the bottom of one module to the bottom of the + module above it, in metres, range [0, 5]. This is the **pitch**, not the gap. + For portrait orientation, this equals module length + gap. For landscape, + module width + gap. If ``number_of_modules_high = 1``, this parameter is ignored. frame_bottom_width : float Frame width at the bottom edge in metres, range [0, 0.5] constant_heat_transfer_coefficient : float @@ -29,7 +42,10 @@ class MountingTypeSpecification(SolarFarmerBaseModel): number_of_modules_long : int or None Number of modules along the rack length (3D only) x_spacing_between_modules : float or None - Horizontal gap between modules in metres, range [0, 5] + Edge-to-edge distance from the left edge of one module to the left edge + of the neighboring module in the same row, in metres, range [0, 5]. + This is the **pitch**, not the gap. For portrait, equals module width + gap. + For landscape, module length + gap. Required for 3D calculations only. frame_top_width : float or None Frame width at the top edge in metres, range [0, 0.5] frame_end_width : float or None @@ -41,11 +57,17 @@ class MountingTypeSpecification(SolarFarmerBaseModel): height_of_tracker_center_from_ground : float or None Tracker rotation axis height in metres (trackers only) transmission_factor : float or None - Bifacial rear-side transmission factor, range [0, 1] + Bifacial rear-side transmission factor, range [0, 1]. **Required** + when the module PAN file has a non-zero ``BifacialityFactor``. + Omitting this field for bifacial modules causes a validation error. bifacial_shade_loss_factor : float or None - Bifacial rear-side shade loss, range [0, 1] + Bifacial rear-side shade loss, range [0, 1]. **Required** when + the module PAN file has a non-zero ``BifacialityFactor``. + Omitting this field for bifacial modules causes a validation error. bifacial_mismatch_loss_factor : float or None - Bifacial rear-side mismatch loss, range [0, 1] + Bifacial rear-side mismatch loss, range [0, 1]. **Required** when + the module PAN file has a non-zero ``BifacialityFactor``. + Omitting this field for bifacial modules causes a validation error. """ is_tracker: bool From 4ea5db3dc20bd3914af4372ed2a162773a799b30 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 15:39:00 -0700 Subject: [PATCH 07/12] Better document calculationYear per-format behavior and weather field cross-references --- .../models/energy_calculation_options.py | 43 ++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/solarfarmer/models/energy_calculation_options.py b/solarfarmer/models/energy_calculation_options.py index eb4e5dd..4557238 100644 --- a/solarfarmer/models/energy_calculation_options.py +++ b/solarfarmer/models/energy_calculation_options.py @@ -21,11 +21,29 @@ class EnergyCalculationOptions(SolarFarmerBaseModel): include_horizon : bool Whether to include horizon shading. Required calculation_year : int - Year to use for the calculation + Year to use for the calculation. Default is 1990. Behavior depends on + meteorological file format: + + - **TSV format**: Ignored — timestamps in the file are used as-is. + - **Meteonorm .dat format**: All timestamps are replaced with this year + while preserving month/day/hour. Used for calculating solar position. + - **PVsyst CSV format with TMY data**: If the file header contains + ``#TMY hourly data``, timestamps are replaced with this year. If the + header is ``#Meteo hourly data`` (non-TMY), original years are preserved. + + **Warning for TMY files**: When using typical meteorological year (TMY) + data with mixed years per month (common in NSRDB, PVGIS datasets), ensure + the file is properly formatted with the ``#TMY hourly data`` header, or + manually remap all timestamps to a single year. Otherwise, only rows + matching ``calculation_year`` may be processed, causing silent partial-year + calculations with incorrect results. default_wind_speed : float Wind speed (m/s) when met data has no wind calculate_dhi : bool - Whether to calculate DHI from GHI + Whether to calculate diffuse horizontal irradiance (DHI) from global + horizontal irradiance (GHI) when the ``DHI`` column is missing from the + meteorological file. If ``False`` and ``DHI`` is missing, the calculation + will fail. Set to ``True`` when using weather files that only provide GHI. apply_diffuse_to_horizon : bool or None Apply diffuse irradiance to the horizon model. Engine default is True horizon_type : HorizonType or None @@ -69,7 +87,15 @@ class EnergyCalculationOptions(SolarFarmerBaseModel): solar_zenith_angle_limit : float or None Maximum solar zenith angle in degrees, range [75, 89.9] missing_met_data_handling : MissingMetDataMethod or None - How to handle missing meteorological data + How to handle missing meteorological data (NaN values or empty cells in + required columns: ``GHI``, ``DHI``, ``TAmb``, ``WS``). Options: + + - ``MissingMetDataMethod.FAIL_ON_VALIDATION``: Raise an error if any + required data is missing. + - ``MissingMetDataMethod.REMOVE_TIMESTAMP``: Skip timesteps with missing + data and continue the calculation. + + If ``None`` (default), the engine uses ``FAIL_ON_VALIDATION``. return_pv_syst_format_time_series_results : bool Return PVsyst-format time-series results return_detailed_time_series_results : bool @@ -81,9 +107,16 @@ class EnergyCalculationOptions(SolarFarmerBaseModel): choice_columns_order_pv_syst_format_time_series : OrderColumnsPvSystFormatTimeSeries or None Column ordering for PVsyst-format output use_albedo_from_met_data_when_available : bool or None - Use albedo from met data when available. Engine default is True + Use albedo from the meteorological file's ``Albedo`` column when available, + instead of the ``MonthlyAlbedo`` payload values. If ``True`` and the + ``Albedo`` column is present, monthly albedo values are ignored. If the + column is absent, falls back to ``MonthlyAlbedo``. Engine default is ``True``. use_soiling_from_met_data_when_available : bool or None - Use soiling from met data when available. Engine default is True + Use soiling loss from the meteorological file's ``Soiling`` column when + available, instead of the ``MountingTypeSpecification.monthly_soiling_loss`` + values. If ``True`` and the ``Soiling`` column is present, monthly soiling + values are ignored. If the column is absent, falls back to + ``monthly_soiling_loss``. Engine default is ``True``. module_mismatch_bin_width : float or None Bin width for module mismatch deduplication. Advanced setting connector_resistance_bin_width : float or None From b34428b0e7d4777315dec96d5fb627beddc530a9 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 16:47:20 -0700 Subject: [PATCH 08/12] Enriched MeteoFileFormat enum with per-value docstrings and TSV spec link --- solarfarmer/models/enums.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/solarfarmer/models/enums.py b/solarfarmer/models/enums.py index ff90335..0f22de9 100644 --- a/solarfarmer/models/enums.py +++ b/solarfarmer/models/enums.py @@ -62,9 +62,40 @@ class IAMModelTypeForOverride(str, Enum): class MeteoFileFormat(str, Enum): - """Meteorological file format.""" + """Meteorological file format. + + The SDK maps file extensions to the correct multipart upload field + automatically: + + - ``.dat`` → ``tmyFile`` (Meteonorm hourly TMY) + - ``.tsv`` → ``tmyFile`` (SolarFarmer tab-separated values) + - ``.csv`` → ``pvSystStandardFormatFile`` (PVsyst standard format) + - ``.gz`` → ``metDataTransferFile`` (SolarFarmer desktop binary export) + + For TSV column names, units, timestamp format, and accepted header aliases, + see :data:`solarfarmer.weather.TSV_COLUMNS` or the user guide: + https://mysoftware.dnv.com/download/public/renewables/solarfarmer/manuals/latest/UserGuide/DefineClimate/SolarResources.html + """ DAT = "dat" + """Meteonorm PVsyst Hourly TMY format (``.dat``).""" TSV = "tsv" + """SolarFarmer tab-separated values format (``.tsv``). + + Tab-separated, one row per timestep. Timestamp format is + ``YYYY-MM-DDThh:mm+OO:OO`` (ISO 8601, mandatory UTC offset, + ``T`` separator required, no seconds component). Example:: + + DateTime GHI DHI Temp Water Pressure Albedo Soiling + 2011-02-02T13:40+00:00 1023.212 1175.619 23.123 1.4102 997 0.2 0.01 + 2011-02-02T13:50+00:00 1026.319 1175.092 23.322 2.0391 997 0.2 0.02 + 2011-02-02T14:00+00:00 871.987 1008.851 23.764 8.9167 1004 0.2 0.03 + + See :data:`solarfarmer.weather.TSV_COLUMNS` for the full column + specification including required/optional columns, units, ranges, + and accepted header aliases. + """ PVSYST_STANDARD_FORMAT = "PvSystStandardFormat" + """PVsyst standard CSV export format (``.csv``).""" PROTOBUF_GZ = "ProtobufGz" + """SolarFarmer desktop binary transfer format (protobuf, gzip-compressed, ``.gz``).""" From 15ad45923a364f540eb54ded164ebab77dc7a853 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 16:55:36 -0700 Subject: [PATCH 09/12] Added TSV_COLUMNS weather file format specification --- solarfarmer/__init__.py | 2 + solarfarmer/weather.py | 101 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 solarfarmer/weather.py diff --git a/solarfarmer/__init__.py b/solarfarmer/__init__.py index c88680b..7d0ba05 100644 --- a/solarfarmer/__init__.py +++ b/solarfarmer/__init__.py @@ -26,6 +26,7 @@ from .endpoint_terminate_async import terminate_calculation from .api import SolarFarmerAPIError from .logging import configure_logging +from .weather import TSV_COLUMNS from .models import ( AuxiliaryLosses, CalculationResults, @@ -108,6 +109,7 @@ "SolarFarmerAPIError", "terminate_calculation", "TrackerSystem", + "TSV_COLUMNS", "Transformer", "TransformerLossModelTypes", "TransformerSpecification", diff --git a/solarfarmer/weather.py b/solarfarmer/weather.py new file mode 100644 index 0000000..430519f --- /dev/null +++ b/solarfarmer/weather.py @@ -0,0 +1,101 @@ +"""SolarFarmer TSV weather file format specification. + +:data:`TSV_COLUMNS` documents the SolarFarmer tab-separated values (TSV) +meteorological file format: required and optional columns, units, valid +ranges, missing-value sentinel, and accepted header aliases. + +Example (tab-separated, from SF-Core test data):: + + DateTime GHI DHI Temp Water Pressure Albedo Soiling + 2011-02-02T13:40+00:00 1023.212 1175.619 23.123 1.4102 997 0.2 0.01 + 2011-02-02T13:50+00:00 1026.319 1175.092 23.322 2.0391 997 0.2 0.02 + 2011-02-02T14:00+00:00 871.987 1008.851 23.764 8.9167 1004 0.2 0.03 + +Timestamp format: ``YYYY-MM-DDThh:mm+OO:OO`` — mandatory UTC offset, +``T`` separator required, no seconds (e.g. ``2011-02-02T13:40+00:00``). +""" + +__all__ = ["TSV_COLUMNS"] + +TSV_COLUMNS: dict = { + # Required columns — parser raises FormatException if absent. + # DateTime must be the first column; others may appear in any order. + "required": [ + { + "name": "DateTime", + "unit": None, + "format": "YYYY-MM-DDThh:mm+OO:OO", + "note": "ISO 8601 with mandatory UTC offset; T separator required; no seconds. Must be the first column.", + "aliases": ["Date", "DateTime", "Time"], + }, + { + "name": "GHI", + "unit": "W/m²", + "range": (0, 1300), + "note": "Required unless POA is provided instead.", + "aliases": ["GHI", "ghi", "Glob", "SolarTotal"], + }, + { + "name": "TAmb", + "unit": "°C", + "range": (-35, 60), + "aliases": ["TAmb", "Temp", "T"], + }, + ], + # Optional columns — omitting them is accepted. + "optional": [ + { + "name": "DHI", + "unit": "W/m²", + "range": (0, 1000), + "note": ( + "May be omitted only when EnergyCalculationOptions.calculate_dhi=True; " + "measured DHI is strongly preferred over engine decomposition." + ), + "aliases": ["DHI", "diff", "diffuse", "DIF"], + }, + { + "name": "WS", + "unit": "m/s", + "range": (0, 50), + "aliases": ["WS", "Wind", "Speed", "WindSpeed"], + }, + { + "name": "Pressure", + "unit": "mbar", + "range": (750, 1100), + "note": "Millibar — not Pa, not hPa.", + "aliases": ["Pressure", "AP", "pressure"], + }, + { + "name": "Water", + "unit": "cm", + "range": (0, 100), + "aliases": ["Water", "PW"], + }, + { + "name": "RH", + "unit": "%", + "range": (0, 100), + "aliases": ["RH", "Humidity", "humidity"], + }, + { + "name": "Albedo", + "unit": None, + "range": (0, 1), + "note": "Used when EnergyCalculationOptions.use_albedo_from_met_data_when_available=True.", + "aliases": ["Albedo", "Surface Albedo", "albedo"], + }, + { + "name": "Soiling", + "unit": None, + "range": (0, 1), + "note": "Used when EnergyCalculationOptions.use_soiling_from_met_data_when_available=True.", + "aliases": ["Soiling", "SL", "SoilingLoss", "Slg"], + }, + ], + "delimiter": "\t", + # Both 9999 and -9999 are treated as missing; 9999.0 / -9999.000 also accepted. + # Behaviour on missing data is controlled by EnergyCalculationOptions.missing_met_data_handling. + "missing_value_sentinel": 9999, +} From 3fe7f4218186f6e43310d0167268a71d2e2ff02a Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 17:00:02 -0700 Subject: [PATCH 10/12] Add doc about sentinal value --- solarfarmer/models/energy_calculation_options.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/solarfarmer/models/energy_calculation_options.py b/solarfarmer/models/energy_calculation_options.py index 4557238..5d7ce0d 100644 --- a/solarfarmer/models/energy_calculation_options.py +++ b/solarfarmer/models/energy_calculation_options.py @@ -87,8 +87,9 @@ class EnergyCalculationOptions(SolarFarmerBaseModel): solar_zenith_angle_limit : float or None Maximum solar zenith angle in degrees, range [75, 89.9] missing_met_data_handling : MissingMetDataMethod or None - How to handle missing meteorological data (NaN values or empty cells in - required columns: ``GHI``, ``DHI``, ``TAmb``, ``WS``). Options: + How to handle missing meteorological data (NaN values, empty cells, or + the sentinel value ``9999`` / ``-9999`` in TSV files) in required columns: + ``GHI``, ``DHI``, ``TAmb``, ``WS``. Options: - ``MissingMetDataMethod.FAIL_ON_VALIDATION``: Raise an error if any required data is missing. @@ -96,6 +97,7 @@ class EnergyCalculationOptions(SolarFarmerBaseModel): data and continue the calculation. If ``None`` (default), the engine uses ``FAIL_ON_VALIDATION``. + See :data:`solarfarmer.weather.TSV_COLUMNS` for TSV sentinel value details. return_pv_syst_format_time_series_results : bool Return PVsyst-format time-series results return_detailed_time_series_results : bool From 26f2c18a09f923dac8279229fc8c029d83533b5e Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 17:21:51 -0700 Subject: [PATCH 11/12] Fix linting errors --- solarfarmer/__init__.py | 4 ++-- solarfarmer/models/pv_plant.py | 2 +- tests/test_endpoint_modelchain.py | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/solarfarmer/__init__.py b/solarfarmer/__init__.py index 7d0ba05..14038c2 100644 --- a/solarfarmer/__init__.py +++ b/solarfarmer/__init__.py @@ -1,4 +1,5 @@ from .__version__ import __version__ +from .api import SolarFarmerAPIError from .config import ( ABOUT_ENDPOINT_URL, ANNUAL_MONTHLY_RESULTS_FILENAME, @@ -24,9 +25,7 @@ from .endpoint_modelchains import run_energy_calculation from .endpoint_service import service from .endpoint_terminate_async import terminate_calculation -from .api import SolarFarmerAPIError from .logging import configure_logging -from .weather import TSV_COLUMNS from .models import ( AuxiliaryLosses, CalculationResults, @@ -56,6 +55,7 @@ TransformerSpecification, ValidationMessage, ) +from .weather import TSV_COLUMNS __all__ = [ "__version__", diff --git a/solarfarmer/models/pv_plant.py b/solarfarmer/models/pv_plant.py index 4e8d328..6d47ca2 100644 --- a/solarfarmer/models/pv_plant.py +++ b/solarfarmer/models/pv_plant.py @@ -43,7 +43,7 @@ class PVPlant(SolarFarmerBaseModel): auxiliary_losses: AuxiliaryLosses | None = None @model_validator(mode="after") - def _check_transformer_spec_references(self) -> "PVPlant": + def _check_transformer_spec_references(self) -> PVPlant: """Validate that referenced transformer spec IDs exist in transformer_specifications.""" for i, transformer in enumerate(self.transformers): spec_id = transformer.transformer_spec_id diff --git a/tests/test_endpoint_modelchain.py b/tests/test_endpoint_modelchain.py index a6a773c..0996358 100644 --- a/tests/test_endpoint_modelchain.py +++ b/tests/test_endpoint_modelchain.py @@ -5,8 +5,7 @@ import pytest import solarfarmer -from solarfarmer.api import Response -from solarfarmer.api import SolarFarmerAPIError +from solarfarmer.api import Response, SolarFarmerAPIError from solarfarmer.endpoint_modelchains import ( _handle_successful_response, _log_api_failure, From 70c88e966b6148175a4de9a0165aa00e4c96e1c7 Mon Sep 17 00:00:00 2001 From: Ian Tse Date: Mon, 6 Apr 2026 17:24:59 -0700 Subject: [PATCH 12/12] Fix line format --- solarfarmer/models/pv_plant.py | 5 ++++- tests/test_endpoint_modelchain.py | 16 ++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/solarfarmer/models/pv_plant.py b/solarfarmer/models/pv_plant.py index 6d47ca2..2a1ca8a 100644 --- a/solarfarmer/models/pv_plant.py +++ b/solarfarmer/models/pv_plant.py @@ -49,7 +49,10 @@ def _check_transformer_spec_references(self) -> PVPlant: spec_id = transformer.transformer_spec_id if spec_id is None: continue - if self.transformer_specifications is None or spec_id not in self.transformer_specifications: + if ( + self.transformer_specifications is None + or spec_id not in self.transformer_specifications + ): available = sorted(self.transformer_specifications or {}) raise ValueError( f"Transformer {i} references spec ID '{spec_id}' which is " diff --git a/tests/test_endpoint_modelchain.py b/tests/test_endpoint_modelchain.py index 0996358..b20264f 100644 --- a/tests/test_endpoint_modelchain.py +++ b/tests/test_endpoint_modelchain.py @@ -196,9 +196,7 @@ def test_non_completed_status_raises_and_logs(self, status, output, caplog): ) with caplog.at_level(logging.ERROR): with pytest.raises(SolarFarmerAPIError) as exc_info: - _handle_successful_response( - response, 3.0, "proj1", None, None, None, False, False - ) + _handle_successful_response(response, 3.0, "proj1", None, None, None, False, False) assert f"Runtime status = {status}" in caplog.text assert status in str(exc_info.value) if output: @@ -374,7 +372,9 @@ def test_async_endpoint_selection( @patch("solarfarmer.endpoint_modelchains.modelchain_call") @patch("solarfarmer.endpoint_modelchains.check_for_3d_files", return_value=False) @patch("solarfarmer.endpoint_modelchains._resolve_request_payload") - def test_failed_response_logs_and_raises(self, mock_resolve_payload, _mock_check_for_3d, mock_modelchain_call, mock_log_failure): + def test_failed_response_logs_and_raises( + self, mock_resolve_payload, _mock_check_for_3d, mock_modelchain_call, mock_log_failure + ): mock_resolve_payload.return_value = ('{"pvPlant":{}}', []) mock_modelchain_call.return_value = Response( code=500, @@ -386,9 +386,7 @@ def test_failed_response_logs_and_raises(self, mock_resolve_payload, _mock_check ) with pytest.raises(SolarFarmerAPIError) as exc_info: - run_energy_calculation( - inputs_folder_path="/tmp/input-folder", project_id="project-1" - ) + run_energy_calculation(inputs_folder_path="/tmp/input-folder", project_id="project-1") mock_log_failure.assert_called_once() assert exc_info.value.status_code == 500 @@ -414,9 +412,7 @@ def test_failed_response_carries_problem_details( ) with pytest.raises(SolarFarmerAPIError) as exc_info: - run_energy_calculation( - inputs_folder_path="/tmp/input-folder", project_id="project-1" - ) + run_energy_calculation(inputs_folder_path="/tmp/input-folder", project_id="project-1") assert exc_info.value.status_code == 400 assert exc_info.value.problem_details == problem_details