Skip to content

Add deprecation warning for irradiance.king#2783

Open
cbcrespo wants to merge 3 commits into
pvlib:mainfrom
cbcrespo:king-deprecation
Open

Add deprecation warning for irradiance.king#2783
cbcrespo wants to merge 3 commits into
pvlib:mainfrom
cbcrespo:king-deprecation

Conversation

@cbcrespo

Copy link
Copy Markdown
Contributor
  • Closes irradiance.king documentation and output naming is wrong #2636
  • I am familiar with the contributing guidelines
  • I attest that all AI-generated material has been vetted for accuracy and is in compliance with the pvlib license
  • Tests added
  • Updates entries in docs/sphinx/source/reference for API changes.
  • Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for 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`).
  • New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

@kandersolar kandersolar added this to the v0.15.3 milestone Jun 18, 2026
@kandersolar kandersolar added the deprecation Use for issues and PRs which involve deprecations label Jun 18, 2026
@kandersolar

Copy link
Copy Markdown
Member

Some minor edits to the tests are needed in order to prevent cluttering the CI output:

https://github.com/pvlib/pvlib-python/actions/runs/27539135408/job/82221477466?pr=2783#step:9:84

pytest offers a way to ensure that warnings are emitted while also suppressing them from the logs. Check #1401 for an example of how to use it.

Comment thread pvlib/irradiance.py

@deprecated(
since="0.15.2",
removal="",

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.

Suggested change
removal="",
removal="0.17.0",

@gaoflow

gaoflow commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

I checked the red flake8-linter job separately from the warning clutter already noted above. The linter is failing on the two-dot diff with these four items:

pvlib/iotools/pvgis.py:416:45: E231 missing whitespace after ':'
pvlib/location.py:111:62: E231 missing whitespace after ':'
pvlib/location.py:119:67: E231 missing whitespace after ':'
tests/iotools/test_midc.py:3:1: F401 'pytz' imported but unused

This minimal patch clears the flake8 command locally:

diff --git a/pvlib/iotools/pvgis.py b/pvlib/iotools/pvgis.py
@@
-        tzname = pytz.timezone(f'Etc/GMT{-tz:+d}')
+        tzname = pytz.timezone('Etc/GMT{:+d}'.format(-tz))
diff --git a/pvlib/location.py b/pvlib/location.py
@@
-            self._zoneinfo = zoneinfo.ZoneInfo(f"Etc/GMT{-tz_:+d}")
+            self._zoneinfo = zoneinfo.ZoneInfo("Etc/GMT{:+d}".format(-tz_))
@@
-            self._zoneinfo = zoneinfo.ZoneInfo(f"Etc/GMT{-int(tz_):+d}")
+            offset = -int(tz_)
+            self._zoneinfo = zoneinfo.ZoneInfo("Etc/GMT{:+d}".format(offset))
diff --git a/tests/iotools/test_midc.py b/tests/iotools/test_midc.py
@@
 import pandas as pd
 import pytest
-import pytz

I verified the lint with the CI-style command on Python 3.12:

git diff origin/main HEAD -- "*.py" | uv run --python 3.12 --with flake8==5.0.4 flake8 --exclude pvlib/version.py --ignore E201,E241,E226,W503,W504 --max-line-length 79 --diff

I also ran related tests, but my local minimal uv run environment did not register the requests_mock fixture for tests/iotools/test_pvgis.py; before that setup error, tests/iotools/test_midc.py and tests/test_location.py passed as part of the run.

@cwhanse

cwhanse commented Jun 19, 2026

Copy link
Copy Markdown
Member

Has something changed in the CI? I thought the linter (flake8) was set to only check lines that are being changed.

@gaoflow

gaoflow commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

I do not think the CI changed. The flake8 job is still diff-only, but it runs a two-dot diff:

git diff upstream/$GITHUB_BASE_REF HEAD -- "*.py" | flake8 ... --diff

So it checks lines that differ between current upstream/main and the checked-out PR merge commit, not only the files GitHub shows in the PR changed-files view. In this run that two-dot diff includes pvlib/iotools/pvgis.py, pvlib/location.py, and tests/iotools/test_midc.py, while the three-dot PR diff only shows pvlib/irradiance.py.

I verified that locally:

git diff origin/main HEAD -- "*.py"      # includes the lint-failing files
git diff origin/main...HEAD -- "*.py"    # only pvlib/irradiance.py

So the linter is still checking changed diff lines, but the two-dot comparison makes unrelated main-branch drift visible when the PR branch is behind/current-main-divergent. Rebasing/merging current main, or applying the small lint-only patch I posted above, should clear that job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api deprecation Use for issues and PRs which involve deprecations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

irradiance.king documentation and output naming is wrong

5 participants