From 16c0ee1c98d58518f863db416eaef98e4c3bb1ab Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 27 Apr 2024 02:16:27 +0200 Subject: [PATCH 1/8] My approach to the issue --- pvlib/pvsystem.py | 75 ++++++++++++------------ pvlib/tests/test_pvsystem.py | 108 ++++++++++++----------------------- 2 files changed, 73 insertions(+), 110 deletions(-) diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py index 7b2f662b13..edf47ab758 100644 --- a/pvlib/pvsystem.py +++ b/pvlib/pvsystem.py @@ -7,7 +7,7 @@ import functools import io import itertools -import os +from pathlib import Path import inspect from urllib.request import urlopen import numpy as np @@ -1957,10 +1957,10 @@ def calcparams_pvsyst(effective_irradiance, temp_cell, return tuple(pd.Series(a, index=index).rename(None) for a in out) -def retrieve_sam(name=None, path=None): - ''' - Retrieve latest module and inverter info from a local file or the - SAM website. +def retrieve_sam(name_or_path): + """ + Retrieve latest module and inverter info from a file bundled with pvlib, + a path or an URL (like SAM's website). This function will retrieve either: @@ -1973,9 +1973,9 @@ def retrieve_sam(name=None, path=None): Parameters ---------- - name : string, optional - Name can be one of: - + name_or_path : string + Use one of the following strings to retrieve a database bundled with + pvlib: * 'CECMod' - returns the CEC module database * 'CECInverter' - returns the CEC Inverter database * 'SandiaInverter' - returns the CEC Inverter database @@ -1984,8 +1984,7 @@ def retrieve_sam(name=None, path=None): * 'SandiaMod' - returns the Sandia Module database * 'ADRInverter' - returns the ADR Inverter database - path : string, optional - Path to the SAM file. May also be a URL. + Additionally, a path to a CSV file or a URL can be provided. Returns ------- @@ -2030,38 +2029,34 @@ def retrieve_sam(name=None, path=None): CEC_Date NaN CEC_Type Utility Interactive Name: AE_Solar_Energy__AE6_0__277V_, dtype: object - ''' - - if name is not None: - name = name.lower() - data_path = os.path.join( - os.path.dirname(os.path.abspath(__file__)), 'data') - if name == 'cecmod': - csvdata = os.path.join( - data_path, 'sam-library-cec-modules-2019-03-05.csv') - elif name == 'sandiamod': - csvdata = os.path.join( - data_path, 'sam-library-sandia-modules-2015-6-30.csv') - elif name == 'adrinverter': - csvdata = os.path.join( - data_path, 'adr-library-cec-inverters-2019-03-05.csv') - elif name in ['cecinverter', 'sandiainverter']: - # Allowing either, to provide for old code, - # while aligning with current expectations - csvdata = os.path.join( - data_path, 'sam-library-cec-inverters-2019-03-05.csv') - else: - raise ValueError(f'invalid name {name}') - elif path is not None: - if path.startswith('http'): - response = urlopen(path) - csvdata = io.StringIO(response.read().decode(errors='ignore')) + """ + internal_dbs = { + "cecmod": "sam-library-cec-modules-2019-03-05.csv", + "sandiamod": "sam-library-sandia-modules-2015-6-30.csv", + "adrinverter": "adr-library-cec-inverters-2019-03-05.csv", + # Both 'cecinverter' and 'sandiainverter', point to same database + # to provide for old code, while aligning with current expectations + "cecinverter": "sam-library-cec-inverters-2019-03-05.csv", + "sandiainverter": "sam-library-cec-inverters-2019-03-05.csv", + } + name_lwr = name_or_path.lower() + if name_lwr in internal_dbs.keys(): # input is a database name + csvdata_path = Path(__file__).parent.joinpath( + "data", internal_dbs[name_lwr] + ) + else: # input is a path or URL + if name_lwr.startswith("http"): # name so check is not case-sensitive + response = urlopen(name_or_path) # URL is case-sensitive + csvdata_path = io.StringIO(response.read().decode(errors="ignore")) + elif Path(name_or_path).exists(): + csvdata_path = name_or_path else: - csvdata = path - elif name is None and path is None: - raise ValueError("A name or path must be provided!") + raise ValueError( + f"Invalid name {name_or_path} or path does not exist. Allowed " + + f"names are {internal_dbs} or a path to a CSV file or URL." + ) - return _parse_raw_sam_df(csvdata) + return _parse_raw_sam_df(csvdata_path) def _normalize_sam_product_names(names): diff --git a/pvlib/tests/test_pvsystem.py b/pvlib/tests/test_pvsystem.py index 52de0fcc5b..8516994442 100644 --- a/pvlib/tests/test_pvsystem.py +++ b/pvlib/tests/test_pvsystem.py @@ -105,80 +105,48 @@ def test_PVSystem_get_iam_invalid(sapm_module_params, mocker): def test_retrieve_sam_raise_no_parameters(): """ - Raise an exception if no parameters are provided to `retrieve_sam()`. + Raise an exception if an invalid parameter is provided to `retrieve_sam()`. """ - with pytest.raises(ValueError) as error: - pvsystem.retrieve_sam() - assert 'A name or path must be provided!' == str(error.value) - - -def test_retrieve_sam_cecmod(): - """ - Test the expected data is retrieved from the CEC module database. In - particular, check for a known module in the database and check for the - expected keys for that module. - """ - data = pvsystem.retrieve_sam('cecmod') - keys = [ - 'BIPV', - 'Date', - 'T_NOCT', - 'A_c', - 'N_s', - 'I_sc_ref', - 'V_oc_ref', - 'I_mp_ref', - 'V_mp_ref', - 'alpha_sc', - 'beta_oc', - 'a_ref', - 'I_L_ref', - 'I_o_ref', - 'R_s', - 'R_sh_ref', - 'Adjust', - 'gamma_r', - 'Version', - 'STC', - 'PTC', - 'Technology', - 'Bifacial', - 'Length', - 'Width', - ] - module = 'Itek_Energy_LLC_iT_300_HE' - assert module in data - assert set(data[module].keys()) == set(keys) + with pytest.raises(ValueError, match="Invalid name"): + pvsystem.retrieve_sam(name_or_path="this_surely_wont_work.csv") def test_retrieve_sam_cecinverter(): - """ - Test the expected data is retrieved from the CEC inverter database. In - particular, check for a known inverter in the database and check for the - expected keys for that inverter. - """ - data = pvsystem.retrieve_sam('cecinverter') - keys = [ - 'Vac', - 'Paco', - 'Pdco', - 'Vdco', - 'Pso', - 'C0', - 'C1', - 'C2', - 'C3', - 'Pnt', - 'Vdcmax', - 'Idcmax', - 'Mppt_low', - 'Mppt_high', - 'CEC_Date', - 'CEC_Type', - ] - inverter = 'Yaskawa_Solectria_Solar__PVI_5300_208__208V_' - assert inverter in data - assert set(data[inverter].keys()) == set(keys) + """Test the expected keys are retrieved from each database.""" + keys_per_database = { + "cecmod": {'Technology', 'Bifacial', 'STC', 'PTC', 'A_c', 'Length', + 'Width', 'N_s', 'I_sc_ref', 'V_oc_ref', 'I_mp_ref', + 'V_mp_ref', 'alpha_sc', 'beta_oc', 'T_NOCT', 'a_ref', + 'I_L_ref', 'I_o_ref', 'R_s', 'R_sh_ref', 'Adjust', + 'gamma_r', 'BIPV', 'Version', 'Date'}, + "sandiamod": {'Vintage', 'Area', 'Material', 'Cells_in_Series', + 'Parallel_Strings', 'Isco', 'Voco', 'Impo', 'Vmpo', + 'Aisc', 'Aimp', 'C0', 'C1', 'Bvoco', 'Mbvoc', 'Bvmpo', + 'Mbvmp', 'N', 'C2', 'C3', 'A0', 'A1', 'A2', 'A3', 'A4', + 'B0', 'B1', 'B2', 'B3', 'B4', 'B5', 'DTC', 'FD', 'A', + 'B', 'C4', 'C5', 'IXO', 'IXXO', 'C6', 'C7', 'Notes'}, + "adrinverter": {'Manufacturer', 'Model', 'Source', 'Vac', 'Vintage', + 'Pacmax', 'Pnom', 'Vnom', 'Vmin', 'Vmax', + 'ADRCoefficients', 'Pnt', 'Vdcmax', 'Idcmax', + 'MPPTLow', 'MPPTHi', 'TambLow', 'TambHi', 'Weight', + 'PacFitErrMax', 'YearOfData'}, + "cecinverter": {'Vac', 'Paco', 'Pdco', 'Vdco', 'Pso', 'C0', 'C1', 'C2', + 'C3', 'Pnt', 'Vdcmax', 'Idcmax', 'Mppt_low', + 'Mppt_high', 'CEC_Date', 'CEC_Type'} + } # fmt: skip + keys_per_database["sandiainverter"] = keys_per_database["cecinverter"] + module_per_database = { + "cecmod": "Itek_Energy_LLC_iT_300_HE", + "sandiamod": "Canadian_Solar_CS6X_300M__2013_", + "adrinverter": "Sainty_Solar__SSI_4K4U_240V__CEC_2011_", + "cecinverter": "ABB__PVI_3_0_OUTD_S_US__208V_", + "sandiainverter": "ABB__PVI_3_0_OUTD_S_US__208V_" + } + + for database in keys_per_database.keys(): + data = pvsystem.retrieve_sam(database) + assert set(data.index) == keys_per_database[database] + assert module_per_database[database] in data.columns def test_sapm(sapm_module_params): From 4ea8fb0efa915e386c5c0148917679d03bb5355a Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 27 Apr 2024 02:49:16 +0200 Subject: [PATCH 2/8] Deprecate previous parameters --- pvlib/pvsystem.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py index edf47ab758..aebfd8aea7 100644 --- a/pvlib/pvsystem.py +++ b/pvlib/pvsystem.py @@ -1957,7 +1957,7 @@ def calcparams_pvsyst(effective_irradiance, temp_cell, return tuple(pd.Series(a, index=index).rename(None) for a in out) -def retrieve_sam(name_or_path): +def retrieve_sam(name_or_path=None, name=None, path=None): """ Retrieve latest module and inverter info from a file bundled with pvlib, a path or an URL (like SAM's website). @@ -1986,6 +1986,16 @@ def retrieve_sam(name_or_path): Additionally, a path to a CSV file or a URL can be provided. + name : string, optional + Name of the database to use. + .. deprecated:: 0.10.5 + Use ``name_or_path`` instead. + + path : string, optional + Path to a CSV file or a URL. + .. deprecated:: 0.10.5 + Use ``name_or_path`` instead. + Returns ------- samfile : DataFrame @@ -2030,6 +2040,22 @@ def retrieve_sam(name_or_path): CEC_Type Utility Interactive Name: AE_Solar_Energy__AE6_0__277V_, dtype: object """ + # error: path was previously silently ignored if name was given GH#2018 + if name is not None and path is not None: + raise ValueError("Please only provide 'name_or_path', not both.") + # deprecation warning for name and path if given GH#2018 + if name is not None or path is not None: + warn_deprecated( + since="0.10.5", + removal="0.11.0", + message=( + "'name' and 'path' parameters have been deprecated in " + + "favor of 'name_or_path' parameter." + ), + name="pvlib.pvsystem.retrieve_sam", + obj_type="parameters 'name' and 'path'", + ) + name_or_path = name or path internal_dbs = { "cecmod": "sam-library-cec-modules-2019-03-05.csv", "sandiamod": "sam-library-sandia-modules-2015-6-30.csv", From a2b446f813e3594cd45e06829aef083345c98a73 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 27 Apr 2024 03:05:16 +0200 Subject: [PATCH 3/8] No reason to over-engineer, right? --- pvlib/pvsystem.py | 88 ++++++++++++++++-------------------- pvlib/tests/test_pvsystem.py | 10 +++- 2 files changed, 47 insertions(+), 51 deletions(-) diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py index aebfd8aea7..2faa8d66ec 100644 --- a/pvlib/pvsystem.py +++ b/pvlib/pvsystem.py @@ -1957,7 +1957,7 @@ def calcparams_pvsyst(effective_irradiance, temp_cell, return tuple(pd.Series(a, index=index).rename(None) for a in out) -def retrieve_sam(name_or_path=None, name=None, path=None): +def retrieve_sam(name=None, path=None): """ Retrieve latest module and inverter info from a file bundled with pvlib, a path or an URL (like SAM's website). @@ -1971,9 +1971,12 @@ def retrieve_sam(name_or_path=None, name=None, path=None): and return it as a pandas DataFrame. + .. note:: + Only provide one of ``name`` or ``path``. + Parameters ---------- - name_or_path : string + name : string, optional Use one of the following strings to retrieve a database bundled with pvlib: * 'CECMod' - returns the CEC module database @@ -1984,17 +1987,8 @@ def retrieve_sam(name_or_path=None, name=None, path=None): * 'SandiaMod' - returns the Sandia Module database * 'ADRInverter' - returns the ADR Inverter database - Additionally, a path to a CSV file or a URL can be provided. - - name : string, optional - Name of the database to use. - .. deprecated:: 0.10.5 - Use ``name_or_path`` instead. - path : string, optional Path to a CSV file or a URL. - .. deprecated:: 0.10.5 - Use ``name_or_path`` instead. Returns ------- @@ -2006,7 +2000,13 @@ def retrieve_sam(name_or_path=None, name=None, path=None): Raises ------ ValueError - If no name or path is provided. + If no ``name`` or ``path`` is provided. + ValueError + If both ``name`` and ``path`` are provided. + ValueError + If the provided ``name`` is not valid. + ValueError + If the provided ``path`` does not exist. Notes ----- @@ -2042,46 +2042,36 @@ def retrieve_sam(name_or_path=None, name=None, path=None): """ # error: path was previously silently ignored if name was given GH#2018 if name is not None and path is not None: - raise ValueError("Please only provide 'name_or_path', not both.") - # deprecation warning for name and path if given GH#2018 - if name is not None or path is not None: - warn_deprecated( - since="0.10.5", - removal="0.11.0", - message=( - "'name' and 'path' parameters have been deprecated in " - + "favor of 'name_or_path' parameter." - ), - name="pvlib.pvsystem.retrieve_sam", - obj_type="parameters 'name' and 'path'", - ) - name_or_path = name or path - internal_dbs = { - "cecmod": "sam-library-cec-modules-2019-03-05.csv", - "sandiamod": "sam-library-sandia-modules-2015-6-30.csv", - "adrinverter": "adr-library-cec-inverters-2019-03-05.csv", - # Both 'cecinverter' and 'sandiainverter', point to same database - # to provide for old code, while aligning with current expectations - "cecinverter": "sam-library-cec-inverters-2019-03-05.csv", - "sandiainverter": "sam-library-cec-inverters-2019-03-05.csv", - } - name_lwr = name_or_path.lower() - if name_lwr in internal_dbs.keys(): # input is a database name - csvdata_path = Path(__file__).parent.joinpath( - "data", internal_dbs[name_lwr] - ) - else: # input is a path or URL - if name_lwr.startswith("http"): # name so check is not case-sensitive - response = urlopen(name_or_path) # URL is case-sensitive - csvdata_path = io.StringIO(response.read().decode(errors="ignore")) - elif Path(name_or_path).exists(): - csvdata_path = name_or_path + raise ValueError("Please provide either 'name' or 'path', not both.") + elif name is None and path is None: + raise ValueError("Please provide either 'name' or 'path'.") + elif name is not None: + internal_dbs = { + "cecmod": "sam-library-cec-modules-2019-03-05.csv", + "sandiamod": "sam-library-sandia-modules-2015-6-30.csv", + "adrinverter": "adr-library-cec-inverters-2019-03-05.csv", + # Both 'cecinverter' and 'sandiainverter', point to same database + # to provide for old code, while aligning with current expectations + "cecinverter": "sam-library-cec-inverters-2019-03-05.csv", + "sandiainverter": "sam-library-cec-inverters-2019-03-05.csv", + } + name_lwr = name.lower() + if name_lwr in internal_dbs.keys(): # input is a database name + csvdata_path = Path(__file__).parent.joinpath( + "data", internal_dbs[name_lwr] + ) else: raise ValueError( - f"Invalid name {name_or_path} or path does not exist. Allowed " - + f"names are {internal_dbs} or a path to a CSV file or URL." + f"Invalid name {name}. Provide one of {internal_dbs.keys()}." ) - + else: # path is not None + if path.lower().startswith("http"): # URL check is not case-sensitive + response = urlopen(path) # URL is case-sensitive + csvdata_path = io.StringIO(response.read().decode(errors="ignore")) + elif Path(path).exists(): + csvdata_path = path + else: + raise ValueError(f"Invalid path {path}: does not exist.") return _parse_raw_sam_df(csvdata_path) diff --git a/pvlib/tests/test_pvsystem.py b/pvlib/tests/test_pvsystem.py index 8516994442..842689f3b0 100644 --- a/pvlib/tests/test_pvsystem.py +++ b/pvlib/tests/test_pvsystem.py @@ -103,12 +103,18 @@ def test_PVSystem_get_iam_invalid(sapm_module_params, mocker): system.get_iam(45, iam_model='not_a_model') -def test_retrieve_sam_raise_no_parameters(): +def test_retrieve_sam_raises_exceptions(): """ Raise an exception if an invalid parameter is provided to `retrieve_sam()`. """ + with pytest.raises(ValueError, match="Please provide either"): + pvsystem.retrieve_sam() + with pytest.raises(ValueError, match="Please provide either.*, not both."): + pvsystem.retrieve_sam(name="this_surely_wont_work", path="wont_work") with pytest.raises(ValueError, match="Invalid name"): - pvsystem.retrieve_sam(name_or_path="this_surely_wont_work.csv") + pvsystem.retrieve_sam(name="this_surely_wont_work") + with pytest.raises(ValueError, match="Invalid path"): + pvsystem.retrieve_sam(path="this_surely_wont_work.csv") def test_retrieve_sam_cecinverter(): From 405de692a55a40a630e3401e4ee811b80df7268e Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 27 Apr 2024 03:15:52 +0200 Subject: [PATCH 4/8] Update v0.10.5.rst --- docs/sphinx/source/whatsnew/v0.10.5.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/sphinx/source/whatsnew/v0.10.5.rst b/docs/sphinx/source/whatsnew/v0.10.5.rst index fe64879c8a..942af697e6 100644 --- a/docs/sphinx/source/whatsnew/v0.10.5.rst +++ b/docs/sphinx/source/whatsnew/v0.10.5.rst @@ -15,6 +15,9 @@ Enhancements Bug fixes ~~~~~~~~~ +* Fixed :py:func:`pvlib.pvsystem.retrieve_sam` silently ignoring the `path` parameter + when `name` was provided. Now an exception is raised requesting to only provide one + of the two parameters. (:issue:`2018`, :pull:`2020`) Testing From 5b4aa4bfa18acaaa09e3318f4d914b39b71863cb Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sat, 27 Apr 2024 03:32:37 +0200 Subject: [PATCH 5/8] Update pvsystem.py --- pvlib/pvsystem.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py index 2faa8d66ec..3f096f39ed 100644 --- a/pvlib/pvsystem.py +++ b/pvlib/pvsystem.py @@ -1979,6 +1979,7 @@ def retrieve_sam(name=None, path=None): name : string, optional Use one of the following strings to retrieve a database bundled with pvlib: + * 'CECMod' - returns the CEC module database * 'CECInverter' - returns the CEC Inverter database * 'SandiaInverter' - returns the CEC Inverter database From bce44f457631fbae27580b96b419f37ede6e01b9 Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Sun, 28 Apr 2024 23:33:55 +0200 Subject: [PATCH 6/8] Improve error handling --- pvlib/pvsystem.py | 21 ++++++++------------- pvlib/tests/test_pvsystem.py | 9 +++++---- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py index 3f096f39ed..53f682a905 100644 --- a/pvlib/pvsystem.py +++ b/pvlib/pvsystem.py @@ -2004,10 +2004,8 @@ def retrieve_sam(name=None, path=None): If no ``name`` or ``path`` is provided. ValueError If both ``name`` and ``path`` are provided. - ValueError - If the provided ``name`` is not valid. - ValueError - If the provided ``path`` does not exist. + KeyError + If the provided ``name`` is not a valid database name. Notes ----- @@ -2056,23 +2054,20 @@ def retrieve_sam(name=None, path=None): "cecinverter": "sam-library-cec-inverters-2019-03-05.csv", "sandiainverter": "sam-library-cec-inverters-2019-03-05.csv", } - name_lwr = name.lower() - if name_lwr in internal_dbs.keys(): # input is a database name + try: csvdata_path = Path(__file__).parent.joinpath( - "data", internal_dbs[name_lwr] + "data", internal_dbs[name.lower()] ) - else: - raise ValueError( + except KeyError: + raise KeyError( f"Invalid name {name}. Provide one of {internal_dbs.keys()}." - ) + ) from None else: # path is not None if path.lower().startswith("http"): # URL check is not case-sensitive response = urlopen(path) # URL is case-sensitive csvdata_path = io.StringIO(response.read().decode(errors="ignore")) - elif Path(path).exists(): - csvdata_path = path else: - raise ValueError(f"Invalid path {path}: does not exist.") + csvdata_path = path return _parse_raw_sam_df(csvdata_path) diff --git a/pvlib/tests/test_pvsystem.py b/pvlib/tests/test_pvsystem.py index 842689f3b0..96cf2f3516 100644 --- a/pvlib/tests/test_pvsystem.py +++ b/pvlib/tests/test_pvsystem.py @@ -111,9 +111,9 @@ def test_retrieve_sam_raises_exceptions(): pvsystem.retrieve_sam() with pytest.raises(ValueError, match="Please provide either.*, not both."): pvsystem.retrieve_sam(name="this_surely_wont_work", path="wont_work") - with pytest.raises(ValueError, match="Invalid name"): + with pytest.raises(KeyError, match="Invalid name"): pvsystem.retrieve_sam(name="this_surely_wont_work") - with pytest.raises(ValueError, match="Invalid path"): + with pytest.raises(FileNotFoundError): pvsystem.retrieve_sam(path="this_surely_wont_work.csv") @@ -140,14 +140,15 @@ def test_retrieve_sam_cecinverter(): 'C3', 'Pnt', 'Vdcmax', 'Idcmax', 'Mppt_low', 'Mppt_high', 'CEC_Date', 'CEC_Type'} } # fmt: skip - keys_per_database["sandiainverter"] = keys_per_database["cecinverter"] module_per_database = { "cecmod": "Itek_Energy_LLC_iT_300_HE", "sandiamod": "Canadian_Solar_CS6X_300M__2013_", "adrinverter": "Sainty_Solar__SSI_4K4U_240V__CEC_2011_", "cecinverter": "ABB__PVI_3_0_OUTD_S_US__208V_", - "sandiainverter": "ABB__PVI_3_0_OUTD_S_US__208V_" } + # duplicate the cecinverter items for sandiainverter, for backwards compat + keys_per_database["sandiainverter"] = keys_per_database["cecinverter"] + module_per_database["sandiainverter"] = module_per_database["cecinverter"] for database in keys_per_database.keys(): data = pvsystem.retrieve_sam(database) From 215c0384f01b3fa878861c6ab2131319b4780b9e Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Mon, 29 Apr 2024 08:38:09 +0200 Subject: [PATCH 7/8] Add ppl involved --- docs/sphinx/source/whatsnew/v0.10.5.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/sphinx/source/whatsnew/v0.10.5.rst b/docs/sphinx/source/whatsnew/v0.10.5.rst index 942af697e6..ff88d2c972 100644 --- a/docs/sphinx/source/whatsnew/v0.10.5.rst +++ b/docs/sphinx/source/whatsnew/v0.10.5.rst @@ -38,3 +38,5 @@ Requirements Contributors ~~~~~~~~~~~~ * Cliff Hansen (:ghuser:`cwhanse`) +* :ghuser:`apct69` +* Mark Mikofski (:ghuser:`mikofski`) From a8d1d153ce4f7d65a90eae27bf10f10d8df6149f Mon Sep 17 00:00:00 2001 From: echedey-ls <80125792+echedey-ls@users.noreply.github.com> Date: Mon, 29 Apr 2024 16:10:14 +0200 Subject: [PATCH 8/8] kevin's suggestions --- pvlib/pvsystem.py | 3 ++- pvlib/tests/test_pvsystem.py | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pvlib/pvsystem.py b/pvlib/pvsystem.py index 53f682a905..184fafddd5 100644 --- a/pvlib/pvsystem.py +++ b/pvlib/pvsystem.py @@ -2060,7 +2060,8 @@ def retrieve_sam(name=None, path=None): ) except KeyError: raise KeyError( - f"Invalid name {name}. Provide one of {internal_dbs.keys()}." + f"Invalid name {name}. " + + f"Provide one of {list(internal_dbs.keys())}." ) from None else: # path is not None if path.lower().startswith("http"): # URL check is not case-sensitive diff --git a/pvlib/tests/test_pvsystem.py b/pvlib/tests/test_pvsystem.py index 96cf2f3516..1151432bd7 100644 --- a/pvlib/tests/test_pvsystem.py +++ b/pvlib/tests/test_pvsystem.py @@ -117,7 +117,7 @@ def test_retrieve_sam_raises_exceptions(): pvsystem.retrieve_sam(path="this_surely_wont_work.csv") -def test_retrieve_sam_cecinverter(): +def test_retrieve_sam_databases(): """Test the expected keys are retrieved from each database.""" keys_per_database = { "cecmod": {'Technology', 'Bifacial', 'STC', 'PTC', 'A_c', 'Length', @@ -140,7 +140,7 @@ def test_retrieve_sam_cecinverter(): 'C3', 'Pnt', 'Vdcmax', 'Idcmax', 'Mppt_low', 'Mppt_high', 'CEC_Date', 'CEC_Type'} } # fmt: skip - module_per_database = { + item_per_database = { "cecmod": "Itek_Energy_LLC_iT_300_HE", "sandiamod": "Canadian_Solar_CS6X_300M__2013_", "adrinverter": "Sainty_Solar__SSI_4K4U_240V__CEC_2011_", @@ -148,12 +148,12 @@ def test_retrieve_sam_cecinverter(): } # duplicate the cecinverter items for sandiainverter, for backwards compat keys_per_database["sandiainverter"] = keys_per_database["cecinverter"] - module_per_database["sandiainverter"] = module_per_database["cecinverter"] + item_per_database["sandiainverter"] = item_per_database["cecinverter"] for database in keys_per_database.keys(): data = pvsystem.retrieve_sam(database) assert set(data.index) == keys_per_database[database] - assert module_per_database[database] in data.columns + assert item_per_database[database] in data.columns def test_sapm(sapm_module_params):