Skip to content

Deprecate pvsystem.singlediode parameter ivcurve_pnts#1743

Merged
kandersolar merged 9 commits into
pvlib:mainfrom
reepoi:singlediode-ivcurvepnts
Jun 23, 2023
Merged

Deprecate pvsystem.singlediode parameter ivcurve_pnts#1743
kandersolar merged 9 commits into
pvlib:mainfrom
reepoi:singlediode-ivcurvepnts

Conversation

@reepoi

@reepoi reepoi commented May 18, 2023

Copy link
Copy Markdown
Contributor

The ivcurve_pnts parameter of pvsystem.singlediode is deprecated in favor of using pvsystem.i_from_v and v_from_i to get additional points on an IV curve. The test cases and docs/examples/iv-modeling/plot_singlediode.py are updated. Also, a bug in tools._golden_sect_DataFrame where scalars were converted into 0d-arrays is fixed.

pvsystem.singlediode returns a dict or a pd.DataFrame. A dict is returned only when all inputs are scalars or ivcurve_pnts > 0.

@kandersolar

Copy link
Copy Markdown
Member

I think ivcurve_pnts deserves a deprecation period before being removed. @reepoi would you be up for modifying this PR to keep the parameter for now, but emit a deprecation warning if it gets used?

@reepoi reepoi force-pushed the singlediode-ivcurvepnts branch from 161522c to 4a1dfab Compare May 23, 2023 17:19
@reepoi reepoi changed the title Remove ivcurve_pnts from pvsystem.singlediode Deprecate pvsystem.singlediode parameter ivcurve_pnts May 23, 2023
@cwhanse cwhanse added this to the 0.10.0 milestone May 31, 2023
Comment thread pvlib/pvsystem.py
Comment thread docs/examples/iv-modeling/plot_singlediode.py Outdated

@cwhanse cwhanse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CI failure is codecov, ignorable.

@kandersolar kandersolar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the late review, I missed that this was tagged 0.10.0! Looks good to me overall; the only thing I see is that these deprecation warnings from the tests need to get cleaned up: https://github.com/pvlib/pvlib-python/actions/runs/5201899928/jobs/9382722429?pr=1743#step:9:69

@reepoi

reepoi commented Jun 13, 2023

Copy link
Copy Markdown
Contributor Author

Ok, I wrapped all the tests using pvsystem.singlediode's ivcurve_pnts parameter with with pytest.warns to hide the warning showing in the pytest output.

Comment thread pvlib/tests/ivtools/test_sde.py Outdated
Comment thread pvlib/tests/ivtools/test_sdm.py Outdated

@kandersolar kandersolar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks @reepoi!

Comment thread pvlib/tests/test_pvsystem.py
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecation Use for issues and PRs which involve deprecations enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants