Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove pvsystem.systemdef #1008

Merged
merged 7 commits into from
Jul 21, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/sphinx/source/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ Other
:toctree: generated/

pvsystem.retrieve_sam
pvsystem.systemdef
pvsystem.scale_voltage_current_power


Expand Down
5 changes: 5 additions & 0 deletions docs/sphinx/source/whatsnew/v0.8.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ API Changes with Deprecations
- :py:func:`pvlib.pvsystem.adrinverter` is now :py:func:`pvlib.inverter.adr`
* Argument ``ac_model`` for :py:class:`pvlib.modelchain.ModelChain` now accepts
``'sandia'``, ``'pvwatts'`` and ``'adr'`` for the inverter models. (:pull:`886`)
* Removed ``systemdef`` function from ``pvsystem.py``. This function was not
used in pvlib and its output was not directly compatible with any pvlib
function. See :py:func:`pvlib.iotools.read_tmy2`,
:py:func:`pvlib.iotools.read_tmy3`, :py:meth:`pvlib.Location.from_tmy`, and
:py:class:`pvlib.pvsystem.LocalizedPVSystem` for alternatives. (:issue:`965`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually the PR is linked as well, though it's easy to get to from the issue page so I don't care about being a stickler about this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this is in the "API Changes with Deprecations" section, but I don't think this was deprecated was it?


API Changes
~~~~~~~~~~~
Expand Down
4,098 changes: 232 additions & 3,866 deletions docs/tutorials/pvsystem.ipynb

Large diffs are not rendered by default.

87 changes: 0 additions & 87 deletions pvlib/pvsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,93 +903,6 @@ def __repr__(self):
('{}: {}'.format(attr, getattr(self, attr)) for attr in attrs)))


def systemdef(meta, surface_tilt, surface_azimuth, albedo, modules_per_string,
strings_per_inverter):
'''
Generates a dict of system parameters used throughout a simulation.

Parameters
----------

meta : dict
meta dict either generated from a TMY file using readtmy2 or
readtmy3, or a dict containing at least the following fields:

=============== ====== ====================
meta field format description
=============== ====== ====================
meta.altitude Float site elevation
meta.latitude Float site latitude
meta.longitude Float site longitude
meta.Name String site name
meta.State String state
meta.TZ Float timezone
=============== ====== ====================

surface_tilt : float or Series
Surface tilt angles in decimal degrees.
The tilt angle is defined as degrees from horizontal
(e.g. surface facing up = 0, surface facing horizon = 90)

surface_azimuth : float or Series
Surface azimuth angles in decimal degrees.
The azimuth convention is defined
as degrees east of north
(North=0, South=180, East=90, West=270).

albedo : float or Series
Ground reflectance, typically 0.1-0.4 for surfaces on Earth
(land), may increase over snow, ice, etc. May also be known as
the reflection coefficient. Must be >=0 and <=1.

modules_per_string : int
Number of modules connected in series in a string.

strings_per_inverter : int
Number of strings connected in parallel.

Returns
-------
Result : dict

A dict with the following fields.

* 'surface_tilt'
* 'surface_azimuth'
* 'albedo'
* 'modules_per_string'
* 'strings_per_inverter'
* 'latitude'
* 'longitude'
* 'tz'
* 'name'
* 'altitude'

See also
--------
pvlib.iotools.read_tmy3
pvlib.iotools.read_tmy2
'''

try:
name = meta['Name']
except KeyError:
name = meta['City']

system = {'surface_tilt': surface_tilt,
'surface_azimuth': surface_azimuth,
'albedo': albedo,
'modules_per_string': modules_per_string,
'strings_per_inverter': strings_per_inverter,
'latitude': meta['latitude'],
'longitude': meta['longitude'],
'tz': meta['TZ'],
'name': name,
'altitude': meta['altitude']}

return system


def calcparams_desoto(effective_irradiance, temp_cell,
alpha_sc, a_ref, I_L_ref, I_o_ref, R_sh_ref, R_s,
EgRef=1.121, dEgdT=-0.0002677,
Expand Down
60 changes: 2 additions & 58 deletions pvlib/tests/test_pvsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,64 +16,7 @@
from pvlib._deprecation import pvlibDeprecationWarning

from conftest import (
needs_numpy_1_10, requires_scipy, fail_on_pvlib_version, DATA_DIR)


def test_systemdef_tmy3():
from pvlib.iotools import tmy
tmy3_testfile = DATA_DIR / '703165TY.csv'
tmy3_data, tmy3_metadata = tmy.read_tmy3(tmy3_testfile)
expected = {'tz': -9.0,
'albedo': 0.1,
'altitude': 7.0,
'latitude': 55.317,
'longitude': -160.517,
'name': '"SAND POINT"',
'strings_per_inverter': 5,
'modules_per_string': 5,
'surface_azimuth': 0,
'surface_tilt': 0}
assert expected == pvsystem.systemdef(tmy3_metadata, 0, 0, .1, 5, 5)


def test_systemdef_tmy2():
from pvlib.iotools import tmy
tmy2_testfile = DATA_DIR / '12839.tm2'
tmy2_data, tmy2_metadata = tmy.read_tmy2(tmy2_testfile)

expected = {'tz': -5,
'albedo': 0.1,
'altitude': 2.0,
'latitude': 25.8,
'longitude': -80.26666666666667,
'name': 'MIAMI',
'strings_per_inverter': 5,
'modules_per_string': 5,
'surface_azimuth': 0,
'surface_tilt': 0}
assert expected == pvsystem.systemdef(tmy2_metadata, 0, 0, .1, 5, 5)


def test_systemdef_dict():
meta = {'latitude': 37.8,
'longitude': -122.3,
'altitude': 10,
'Name': 'Oakland',
'State': 'CA',
'TZ': -8}

# Note that TZ is float, but Location sets tz as string
expected = {'tz': -8,
'albedo': 0.1,
'altitude': 10,
'latitude': 37.8,
'longitude': -122.3,
'name': 'Oakland',
'strings_per_inverter': 5,
'modules_per_string': 5,
'surface_azimuth': 0,
'surface_tilt': 5}
assert expected == pvsystem.systemdef(meta, 5, 0, .1, 5, 5)
needs_numpy_1_10, requires_scipy, fail_on_pvlib_version)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny nit, this import might fit on one line now



@pytest.mark.parametrize('iam_model,model_params', [
Expand Down Expand Up @@ -104,6 +47,7 @@ def test_PVSystem_get_iam_interp(sapm_module_params, mocker):
with pytest.raises(ValueError):
system.get_iam(45, iam_model='interp')


def test__normalize_sam_product_names():

BAD_NAMES = [' -.()[]:+/",', 'Module[1]']
Expand Down