Conversation
|
Reason for importing |
|
no particular reason. Just that I had seen I have replaced |
| spline = make_interp_spline( | ||
| x, y, k=3) |
There was a problem hiding this comment.
| spline = make_interp_spline( | |
| x, y, k=3) | |
| spline = make_interp_spline(x, y, k=3) |
No reason to not fit this on one line, right?
| # Contributed by Anton Driesse (@adriesse), PV Performance Labs. July, 2019 | ||
|
|
||
| from scipy.interpolate import interp1d | ||
| from scipy.interpolate import make_interp_spline |
There was a problem hiding this comment.
This line being here inside the function (instead of at the top of the file as usual) is a relic of the days when scipy was an optional dependency. Optional for this PR, but moving the import to the top would be an improvement.
| fill_value='extrapolate') | ||
| aoi_input = aoi | ||
| theta_ref = np.asarray(theta_ref) | ||
| iam_ref = np.asarray(iam_ref) |
There was a problem hiding this comment.
Why is np.asarray needed here?
| return spline(x) | ||
|
|
||
| else: | ||
| raise ValueError(f"Invalid interpolation method '{method}'.") |
There was a problem hiding this comment.
This is, technically, a breaking change, since the interp1d way also supported 'nearest', 'nearest-up', 'zero', 'slinear', 'previous', and 'next'. I doubt these got much use, if any. Any thoughts on how to handle that?
| spline = make_interp_spline(theta_ref, iam_ref, k=3) | ||
|
|
||
| def interpolator(x): | ||
| return spline(x) |
There was a problem hiding this comment.
This can surely be simplified as @cwhanse mentioned in #2741 (comment)
| Specifies the interpolation method. | ||
| Useful options are: 'linear', 'quadratic', 'cubic'. | ||
| See scipy.interpolate.interp1d for more options. | ||
| See scipy.interpolate for more options. |
There was a problem hiding this comment.
This line may need to be edited, depending on https://github.com/pvlib/pvlib-python/pull/2741/changes#r3137773820
3 participants
scipy.interpolate.interp1dis discouraged #2394docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.The existing tests pass. I have added 2 in test_iam.py under test_iam_interp.