Improve extraradiation input/output types

[wholmgren]

As pointed out by @adriesse in #217, the extraradiation function type handling is abysmal. This PR should fix that. I think it now "just works", but out of laziness, I will only quote the new test code instead of explaining in words what inputs turn into what outputs. I think the only potentially controversial one would be the fact that a Timestamp input yields a TimeSeries output. One might argue that only DatetimeIndex input should yield a TimeSeries output.

timestamp = pd.Timestamp('20161026')
dt_index = pd.DatetimeIndex([timestamp])
doy = timestamp.dayofyear
dt_date = timestamp.date()  # datetime.date object
dt_datetime = datetime.datetime.combine(dt_date, datetime.time(0))
dt_np64 = np.datetime64(dt_datetime)
value = 1383.636203

@pytest.mark.parametrize('input, expected', [
    (doy, value),
    (np.float64(doy), value),
    (dt_date, value),
    (dt_datetime, value),
    (dt_np64, value),
    (np.array([doy]), np.array([value])),
    (pd.Series([doy]), np.array([value])),
    (dt_index, pd.Series([value], index=dt_index)),
    (timestamp, pd.Series([value], index=dt_index))
])
@pytest.mark.parametrize('method', [
    'asce', 'spencer', 'nrel', requires_ephem('pyephem')])
def test_extraradiation(input, expected, method):
    out = irradiance.extraradiation(input)
    assert_allclose(out, expected, atol=1)

This runs 9*4=36 different tests, one for each combination of input type and method. They all pass.

My implementation requires 9 private helper functions, which seems a bit extreme, but I couldn't think of a better way to do it that didn't involve a lot of copy/paste. I am definitely open to suggestions for improvements.

Closes #216. Closes #217.

[wholmgren]

I decided that a scalar Timestamp input should yield a scalar float, not array-like Series, output and updated the code accordingly. I also moved the private helper functions to tools.py since they might be useful in other parts of the library and they were polluting an already too long irradiance module.

I also updated the irradiance tutorial to demonstrate the new behaviors.

I think this is ready to go.

[adriesse]

Well you really went all out on this one! Or maybe it's in line with the rest of the library, I don't know (yet).

My instinctive reaction, which may not be pythonic, is that this goes a bit far in terms of input/output flexibility. Perhaps having only a single or array of floats out is enough? As for the interaction between input type and method, this almost suggests two functions: one for a generic year, and one for a specific year(s). I would leave a little bit more work for the caller that wan'ts to experiment with this particular concept.

[wholmgren]

Yes, I may have gone a bit overboard here, though once it works consistently with array dayofyear and DatetimeIndex for all of the methods, it's actually not too much more effort to make it work with everything else. I think that DatetimeIndex in, TimeSeries-out is nice and I can't think of a situation in which one would not want that. This is similar to how other pvlib functions work, too.

One of the utility functions, _doy_to_timestamp, has a epoch keyword argument that determines the year of a day-of-year to timestamp conversion. Would making a similar keyword argument part of the public API address your concern? I would implement it slightly differently for a public API than the diff currently shows (it would only accept an integer year argument instead of last day of the previous year string).

[wholmgren]

@adriesse I added an epoch_year argument that might satisfy your curiosity. More importantly to me, I reduced the complexity of the code a little bit.

I'll merge this tomorrow.

[adriesse]

Yes, that's good to bring that parameter out. I like reduced complexity
too!

On 2016-07-20 0:00, Will Holmgren wrote:

@adriesse https://github.com/adriesse I added an |epoch_year|
argument that might satisfy your curiosity. More importantly to me, I
reduced the complexity of the code a little bit.

I'll merge this tomorrow.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#219 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AIlYQzGgT__lHdiuaLTJdse6lCdd0XTEks5qXUkJgaJpZM4JN3e4.

Photovoltaic Performance Labs Germany
Emmy-Noether-Str. 2
79110 Freiburg
Germany

+49-761-8973-5603 (Europe)
+49-174-532-7677 (Mobile)
+1-613-817-1652 (North America)

www.pvperformancelabs.com