coerce and rotate pvgis TMY data to desired tz and year #2138

mikofski · 2024-07-19T02:12:17Z

add private function _coerce_and_roll_pvgis_tmy()
add roll_utc_offset and coerce_year params to docstring for get_pvgis_tmy
call private function if roll_utc_offset is not None or coerce_year is not None

Closes add boilerplate to fix pvgis tmy to a timezone and year #2139
I am familiar with the contributing guidelines
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.

- add private function `_coerce_and_rotate_pvgis()` - add `utc_offset` and `coerce_year` params to docstring for `get_pvgis_tmy` - call private function if `utc_offset` is not zero

check if utc_offset and coerce_year work as expected

- remove whitespace - shorter lines

- incorrect syntax for indices

- if february is a leap year, when shifting tz, causes issues - so replace february year with non-leap year

- also fix use ts for timestamp when fixing feb leap year

- lower case "s" not TimeStamp

mikofski · 2024-07-19T08:50:32Z

Tested in this Colab notebook: test_coerce_pvgis.ipynb.

cwhanse · 2024-07-19T16:37:04Z

Showing my ignorance of PVGIS: does it always return 8760 timestamps?

If PVGIS does return 8760, what should I expect if a user coerces to a leap year? I assume they would get a DatetimeIndex with Feb 29 missing.

Second question: if I rotate to UTC-10 (utc_offset=-10), what should I expect as the result? An index of length 8750, or an index of length 8760 but with the last 10 hours in the following year? If its 8760, what data are in the last 10 hours?

mikofski · 2024-07-19T16:54:30Z

get_pvgis_tmy() always returns 8760 in utc
yes, you are correct. If coerced to a leap year then Feb 29 is skipped, this matches other functions in iotools with this argument
if you rotate to utc offset of -10 then the output will be a 8760 starting at midnight of the forced year, the first 10 rows are “rotated” to the bottom, because in utc-10 they are from 2am to 11pm on December 31st. The tests check the output to see that the first timestamp is January 1st at midnight, and the last timestamp is December 31st at 11pm

Where is utc-10 is that Hawaii?

cwhanse · 2024-07-19T17:00:39Z

Where is utc-10 is that Hawaii?

Dunno I just picked a number for illustration.

cwhanse · 2024-07-19T17:01:39Z

if you rotate to utc offset of -10 then the output will be a 8760 starting at midnight of the forced year, the first 10 rows are “rotated” to the bottom, because in utc-10 they are from 2am to 11pm on December 31st.

Ah. Basically we assume a TMY year is periodic, thus every hour's data is defined in the rotated index.

- add description and links to issue/pr

pvlib/iotools/pvgis.py

- also use np.roll - also make new index and dataframe instead of altering original - removes need to sanitize original index February for leap year - remove calendar import but add numpy and pytz - code much simpler shorter, easier to read

pvlib/tests/iotools/test_pvgis.py

- fix Turin is actually CET which is UTC+1 - be DRY so use variables for test year and tz constants, versus WET and hardcoded - check tz unchanged if default zero utc_offset - use _ output args instead of indexing data[0] - add comments

kandersolar

FYI, the PSM3 TMY API does this "roll over" also: https://assessingsolar.github.io/unofficial-psm3-userguide/pages/rollover.html

pvlib/iotools/pvgis.py

adriesse · 2024-07-30T20:11:21Z

FYI, the PSM3 TMY API does this "roll over" also: https://assessingsolar.github.io/unofficial-psm3-userguide/pages/rollover.html

TMY methods have ways of merging between months from different years. This could be done here also.

- explain setting utc_offset will roll data to start at Jan 1 midnight

pvlib/iotools/pvgis.py

- update arg docstrings - allow user to coerce year even if utc_offset is None or zero - use 1990 as default if utc_offset is not None or zero, but coerce_year was unspecified - add warning comment to be explicit and test identity to avoid unexpected implicit booleaness

- refactor utc_offset everywhere including comments and docstring - add additional test to coerce year even if utc offset is zero or none - change tzname to 'UTC' (versus Etc/GMT or Etc/GMT+0) if replacing with zero utc offset

kandersolar

LGTM. We may want to add similar functionality for other TMY datasets in the future, so I think it's worth @AdamRJensen taking a look too if he has the time.

pvlib/iotools/pvgis.py

docs/sphinx/source/whatsnew/v0.11.1.rst

adriesse · 2024-08-01T14:44:06Z

LGTM. We may want to add similar functionality for other TMY datasets in the future,

LGTM2

My perception is that this is fixing a flaw in the PVGIS service. Hopefully other TMY sources align on local time.

use "optional" vs. "default None" per pvlib#1574 Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>

rename argument "roll_utc_offset" in whatsnew Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>

AdamRJensen · 2024-08-05T15:39:47Z

My perception is that this is fixing a flaw in the PVGIS service. Hopefully other TMY sources align on local time.

The PVGIS team is currently reworking the entire process, so this is a good time to make requests about features and provide feedback. Tagging @NikosAlexandris

AdamRJensen

I tested it and it seems to work great 🥳

One thing I noticed was that the index name is maintained as "time(UTC)" when utc_roll_offset is None and is removed when utc_roll_offset is specified. Should this be tested? Maybe it doesn't matter

pvlib/iotools/pvgis.py

- treat tz=None as UTC - allows get_pvgis_tmy to be simpler - remove unnecessary comments

mikofski · 2024-08-06T03:34:04Z

One thing I noticed was that the index name is maintained as "time(UTC)" when utc_roll_offset is None and is removed when utc_roll_offset is specified. Should this be tested? Maybe it doesn't matter

I renamed the index and added tests in 1e9ee64

- ... in docstring of private function pvgis._coerce_and_roll_tmy() - rename tmy_data - name new_index explicitly using pd.DatetimeIndex()

AdamRJensen · 2024-08-16T19:43:09Z

Thanks @mikofski Great to see this new feature in the iotools!

NikosAlexandris · 2024-08-31T13:44:00Z

My perception is that this is fixing a flaw in the PVGIS service. Hopefully other TMY sources align on local time.

The PVGIS team is currently reworking the entire process, so this is a good time to make requests about features and provide feedback. Tagging @NikosAlexandris

Thank you for pinging me here. Will look into this as time permits.

NikosAlexandris · 2024-09-30T18:41:49Z

Tested in this Colab notebook: test_coerce_pvgis.ipynb.

@mikofski Would you have some time to help me better understand the "issue" described in this Notebook ? Thank you.

mikofski · 2024-10-01T02:18:19Z

Hi @NikosAlexandris , in past versions of iotools.get_pvgis_tmy() the data-frame returned was always in UTC time. In my experience TMY files have traditionally been in local time, starting & ending at midnight. Therefore, the “issue” tested in the notebook is if the additional arguments roll_utc_offset and coerce_year will rotate the data frame to coincide with local time, starting at midnight in the desired year

mikofski added 7 commits July 18, 2024 19:10

coerce and rotate pvgis TMY data to desired tz and year

883cfbe

- add private function `_coerce_and_rotate_pvgis()` - add `utc_offset` and `coerce_year` params to docstring for `get_pvgis_tmy` - call private function if `utc_offset` is not zero

test get_pvgis_tmy_coerce_year

7e6e4c0

check if utc_offset and coerce_year work as expected

fix flake8 in test_pvgis_coerce_year

84d4695

- remove whitespace - shorter lines

remove iloc for index in test pvgis coerce

3e42075

- incorrect syntax for indices

deal with leap year in pvgis when coercing

d0bd0a0

- if february is a leap year, when shifting tz, causes issues - so replace february year with non-leap year

fix space around operator in coerce pvgis

ec482ac

- also fix use ts for timestamp when fixing feb leap year

fix pd.Timestamp in pvgis coerce year

883ced8

- lower case "s" not TimeStamp

mikofski marked this pull request as ready for review July 19, 2024 08:50

mikofski self-assigned this Jul 19, 2024

mikofski added the remote-data triggers --remote-data pytests label Jul 19, 2024

mikofski requested a review from AdamRJensen July 19, 2024 09:03

Update v0.11.1 what's new for coerce pvgis tmy

dd2c85f

- add description and links to issue/pr

mikofski added this to the v0.11.1 milestone Jul 20, 2024

mikofski requested review from adriesse and kandersolar July 26, 2024 03:32

adriesse reviewed Jul 26, 2024

View reviewed changes

mikofski added 3 commits July 27, 2024 02:46

remove unused imports from pvgis.py for flake8

72c55f6

change private function name to _coerce_and_roll_pvgis_tmy

0735183

mikofski requested a review from cwhanse July 27, 2024 10:06

cwhanse reviewed Jul 27, 2024

View reviewed changes

pvlib/tests/iotools/test_pvgis.py Outdated Show resolved Hide resolved

cwhanse approved these changes Jul 27, 2024

View reviewed changes

mikofski removed request for AdamRJensen and kandersolar July 28, 2024 00:59

kandersolar reviewed Jul 30, 2024

View reviewed changes

pvlib/iotools/pvgis.py Outdated Show resolved Hide resolved

pvlib/iotools/pvgis.py Outdated Show resolved Hide resolved

Update utc_offset description

a38cf9a

- explain setting utc_offset will roll data to start at Jan 1 midnight

kandersolar added the enhancement label Jul 31, 2024

kandersolar reviewed Jul 31, 2024

View reviewed changes

pvlib/iotools/pvgis.py Outdated Show resolved Hide resolved

mikofski added 2 commits July 31, 2024 11:30

rename roll_utc_offset in get_pvgis_tmy

4fd05fb

- refactor utc_offset everywhere including comments and docstring - add additional test to coerce year even if utc offset is zero or none - change tzname to 'UTC' (versus Etc/GMT or Etc/GMT+0) if replacing with zero utc offset

kandersolar approved these changes Aug 1, 2024

View reviewed changes

pvlib/iotools/pvgis.py Outdated Show resolved Hide resolved

docs/sphinx/source/whatsnew/v0.11.1.rst Outdated Show resolved Hide resolved

mikofski and others added 2 commits August 1, 2024 10:50

Update pvlib/iotools/pvgis.py with suggestions

1b92d40

use "optional" vs. "default None" per pvlib#1574 Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>

Update docs/sphinx/source/whatsnew/v0.11.1.rst

43ce3c4

rename argument "roll_utc_offset" in whatsnew Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>

mikofski requested a review from AdamRJensen August 5, 2024 03:04

AdamRJensen approved these changes Aug 5, 2024

View reviewed changes

pvlib/iotools/pvgis.py Outdated Show resolved Hide resolved

pvlib/iotools/pvgis.py Show resolved Hide resolved

pvlib/iotools/pvgis.py Outdated Show resolved Hide resolved

pvlib/iotools/pvgis.py Outdated Show resolved Hide resolved

AdamRJensen and others added 5 commits August 5, 2024 19:08

Merge branch 'main' into coerce-and-rotate-pvgis

76c8ec6

Merge branch 'main' into coerce-and-rotate-pvgis

d77a3d3

rename _coerce_and_roll_tmy, remove 'pvgis'

91c169e

rename index with new tz in coerce pvgis tmy

1e9ee64

allow tz of None in _coerce_and_roll_tmy

d24e4f3

- treat tz=None as UTC - allows get_pvgis_tmy to be simpler - remove unnecessary comments

mikofski added 4 commits August 12, 2024 19:20

Merge branch 'main' into coerce-and-rotate-pvgis

f12571d

clarify input tmy_data is UTC...

fb2813d

- ... in docstring of private function pvgis._coerce_and_roll_tmy() - rename tmy_data - name new_index explicitly using pd.DatetimeIndex()

fix flake8 in _coerce_and_roll_tmy

ac9dbb0

Merge branch 'main' into coerce-and-rotate-pvgis

9875521

AdamRJensen merged commit b5e89be into pvlib:main Aug 16, 2024
24 of 25 checks passed

mikofski deleted the coerce-and-rotate-pvgis branch August 16, 2024 19:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

coerce and rotate pvgis TMY data to desired tz and year #2138

coerce and rotate pvgis TMY data to desired tz and year #2138

mikofski commented Jul 19, 2024 •

edited

Loading

mikofski commented Jul 19, 2024

cwhanse commented Jul 19, 2024

mikofski commented Jul 19, 2024

cwhanse commented Jul 19, 2024

cwhanse commented Jul 19, 2024

kandersolar left a comment

adriesse commented Jul 30, 2024 •

edited

Loading

kandersolar left a comment

adriesse commented Aug 1, 2024

AdamRJensen commented Aug 5, 2024 •

edited

Loading

AdamRJensen left a comment

mikofski commented Aug 6, 2024

AdamRJensen commented Aug 16, 2024

NikosAlexandris commented Aug 31, 2024

NikosAlexandris commented Sep 30, 2024

mikofski commented Oct 1, 2024 •

edited

Loading

coerce and rotate pvgis TMY data to desired tz and year #2138

coerce and rotate pvgis TMY data to desired tz and year #2138

Conversation

mikofski commented Jul 19, 2024 • edited Loading

mikofski commented Jul 19, 2024

cwhanse commented Jul 19, 2024

mikofski commented Jul 19, 2024

cwhanse commented Jul 19, 2024

cwhanse commented Jul 19, 2024

kandersolar left a comment

Choose a reason for hiding this comment

adriesse commented Jul 30, 2024 • edited Loading

kandersolar left a comment

Choose a reason for hiding this comment

adriesse commented Aug 1, 2024

AdamRJensen commented Aug 5, 2024 • edited Loading

AdamRJensen left a comment

Choose a reason for hiding this comment

mikofski commented Aug 6, 2024

AdamRJensen commented Aug 16, 2024

NikosAlexandris commented Aug 31, 2024

NikosAlexandris commented Sep 30, 2024

mikofski commented Oct 1, 2024 • edited Loading

mikofski commented Jul 19, 2024 •

edited

Loading

adriesse commented Jul 30, 2024 •

edited

Loading

AdamRJensen commented Aug 5, 2024 •

edited

Loading

mikofski commented Oct 1, 2024 •

edited

Loading