Skip to content

Add solar interpolation module - #2386

Open
SarahG-579462 wants to merge 16 commits into
mainfrom
solar_interp
Open

Add solar interpolation module#2386
SarahG-579462 wants to merge 16 commits into
mainfrom
solar_interp

Conversation

@SarahG-579462

@SarahG-579462 SarahG-579462 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGELOG.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Adds a module for interpolating solar noon (and potentially other interesting solar times)

Does this PR introduce a breaking change?

  • No

Other information:

Todo:

  • Module code
  • Optional dependencies
  • Tests
  • Documenting with a notebook?

@Zeitsperre Zeitsperre left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Neat additions!

I think you should rebase on the dev-v099 branch since we're so close to the release for it. If we do put out a final v0.x version, it shouldn't have any brand new modules.

The v1 candidate branch has more subdivided extra dependencies, so it might make sense to add a new one (xclim[solar]?).

Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread pyproject.toml Outdated
@Zeitsperre Zeitsperre added the xclim-v1 First major release of xclim label Jul 22, 2026
@github-actions github-actions Bot added the CI Automation and Contiunous Integration label Jul 23, 2026
@SarahG-579462
SarahG-579462 marked this pull request as ready for review July 23, 2026 22:01

@aulemahal aulemahal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

First pass before leaving for two weeks so I don't need to explain it any further.

Comment thread src/xclim/solar.py Outdated
Comment thread pyproject.toml
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
Comment thread src/xclim/solar.py Outdated
@github-actions github-actions Bot added the indicators Climate indices and indicators label Jul 27, 2026
@SarahG-579462
SarahG-579462 requested a review from Zeitsperre July 27, 2026 20:30

@Zeitsperre Zeitsperre left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is looking really good. I'm going to restate though that this should be added to the first v1 release, given that it adds a bunch of new features and refactors several existing functions into a new (albeit much more sensible) location.

Is there a delivery date involved here or are things OK to stay as a PR for the next month or two?

@SarahG-579462

Copy link
Copy Markdown
Contributor Author

This is looking really good. I'm going to restate though that this should be added to the first v1 release, given that it adds a bunch of new features and refactors several existing functions into a new (albeit much more sensible) location.

Is there a delivery date involved here or are things OK to stay as a PR for the next month or two?

This is fine, I'll just point towards this branch and ask them to install locally.

@@ -0,0 +1,1194 @@
"""The solar module offers functions for interpolating and accumulating variables to solar noon."""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"""The solar module offers functions for interpolating and accumulating variables to solar noon."""
"""The solar module offers helper functions for astronomical calculations related to the sun, its position in the sky, its revolution and to the length of day."""

Comment on lines +21 to +27
try:
from flox.xarray import rechunk_for_blockwise

flox_err = None
except ImportError:
rechunk_for_blockwise = None

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
try:
from flox.xarray import rechunk_for_blockwise
flox_err = None
except ImportError:
rechunk_for_blockwise = None

I believe this was copied from the helpers, but is not actually needed ? I think it's for resample_map which still resides in the other file.

@github-actions github-actions Bot added the approved Approved for additional tests label Aug 13, 2026
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage is 91.806%solar_interp into main. No base build found for main.

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

Labels

approved Approved for additional tests CI Automation and Contiunous Integration indicators Climate indices and indicators xclim-v1 First major release of xclim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants