Skip to content

Fix hour-angle and geometric event conversion on DST transition days - #2923

Open
kadyrbekovhamit-cyber wants to merge 4 commits into
pvlib:mainfrom
kadyrbekovhamit-cyber:codex/h24-pvlib-dst
Open

kadyrbekovhamit-cyber wants to merge 4 commits into
pvlib:mainfrom
kadyrbekovhamit-cyber:codex/h24-pvlib-dst

Conversation

@kadyrbekovhamit-cyber

@kadyrbekovhamit-cyber kadyrbekovhamit-cyber commented Oct 24, 2026 •

Copy link
Copy Markdown

On DST-transition days, elapsed time since local midnight differs from the local clock time. hour_angle already subtracts the UTC offset at each input timestamp, so combining it with elapsed hours can introduce a 15-degree discrepancy. Geometric sunrise/set conversion has the corresponding one-hour shift.

Use naive local wall-clock subtraction for the forward calculation, and construct the inverse UTC instant from the date, wall-clock hours and timestamp-specific offset. This also avoids an ambiguous/nonexistent local midnight. The return types and function signatures remain unchanged. Related #2132/#2133 addressed midnight handling; this proposal addresses the remaining numerical discrepancy.

Regression validation on current main 0c6ab77: six transition/ambiguous-midnight checks fail before and pass after. The complete test_solarposition.py module has 65 passes and 21 skips (optional dependencies unavailable), versus 59 passes, six failures and 21 skips before. Current solarposition.py and tests ran over the existing pvlib installation; not a full source install or full project suite. No measured annual-production or financial-loss claim.

Prepared by Xamit Kadirbekov and Daniyal Kadirbekov with Codex/ChatGPT assistance for investigation, patch, tests and writing. Synthetic tests use a direct UTC hour-angle equation and timezone-invariance checks.

  • No new issue; related NonExistentTimeError when calling hour_angle #2132 and Handle time normalization for nonexistent and ambiguous times #2133, which are not reopened or claimed as resolved here.
  • 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.

Documentation CI follow-up (24 October)

Read the Docs build 35380797 failed. Its raw log reports ten gallery-example failures in pvlib/iotools/tmy.py:226, ValueError: Cannot modify read-only array, plus downstream undefined-variable errors in the clearsky guide. The tmy.py Git blob is identical on the base and this branch. I have not run a complete documentation build on the base, so I am not claiming that the whole CI pipeline is clean. The scoped solar-position regression results above remain unchanged.

@github-actions

Copy link
Copy Markdown

Hey @kadyrbekovhamit-cyber! 🎉

Thanks for opening your first pull request! We appreciate your
contribution. Please ensure you have reviewed and understood the
contributing guidelines.

If AI is used for any portion of this PR, you must vet the content
for technical accuracy.

Finally, be sure the PR description includes the PR
checklist,
and complete the items you are able to.

kadyrbekovhamit-cyber and others added 3 commits October 24, 2026 22:10
Bring in pandas 3.1 compatibility (pvlib#2924), which fixes the read_tmy3
failure that broke the Read the Docs gallery build of this PR.

Co-authored-by: Xamit Kadirbekov <kadyrbekovkhamit@gmail.com>
Co-authored-by: Daniyal Kadirbekov <daniyal.kadirbekov@gmail.com>
Signed-off-by: kadyrbekovhamit-cyber <288885044+kadyrbekovhamit-cyber@users.noreply.github.com>
- use DatetimeIndex.day_of_year in the new DST test, as pvlib#2924 does;
- drop the _AmbiguousTimeError alias that the updated
  test_hour_angle_with_tricky_timezones no longer uses;
- list the contributors one per line in the v0.16.2 what's new.

Co-authored-by: Xamit Kadirbekov <kadyrbekovkhamit@gmail.com>
Co-authored-by: Daniyal Kadirbekov <daniyal.kadirbekov@gmail.com>
Signed-off-by: kadyrbekovhamit-cyber <288885044+kadyrbekovhamit-cyber@users.noreply.github.com>
@kadyrbekovhamit-cyber

Copy link
Copy Markdown
Author

I merged current main (0f6631a) into the branch to resolve the conflict. There was no force-push; the branch history is unchanged.

  • Docs build. The earlier Read the Docs failure was not caused by this PR. All ten gallery failures came from read_tmy3(..., coerce_year=...) raising ValueError: Cannot modify read-only array under pandas 3.1.0 (tmy.py:226), and the clearsky-guide errors followed from that. main fixed this in [DEV] address pandas 3.1.0 compatibility issues #2924, and the RTD builds of [DEV] address pandas 3.1.0 compatibility issues #2924/Link pvlib.iotools in the FAQ entry on irradiance data #2925/Address flake8 warnings introduced in #2924 #2926 pass. With the merge, read_tmy3(coerce_year=1990) now works locally with the RTD versions (pandas 3.1.0, numpy 2.5.4). I did not build the full docs locally, so the new RTD run is the real check.
  • Follow-ups in the same push. In the new test, dayofyear → day_of_year, to match [DEV] address pandas 3.1.0 compatibility issues #2924. I removed the _AmbiguousTimeError alias in test_hour_angle_with_tricky_timezones, which this PR no longer uses. In the 0.16.2 "what's new", the contributor entry is now one name per line in the normal list.
  • Tests (pandas 3.1.0, numpy 2.5.4). tests/test_solarposition.py: 65 passed, 21 skipped (optional solvers not installed). With solarposition.py reverted to main, the new DST tests fail: 6 failed, 59 passed, 21 skipped. tests/iotools/test_tmy.py: 9 passed. Before the merge, test_read_tmy3_coerce_year and test_gh865_read_tmy3_feb_leapyear_hr24 failed with the same read-only-array error as the docs build. flake8 reports nothing new on the changed lines.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant