Skip to content

Add support for return_components to irradiance.isotropic#2787

Open
cbcrespo wants to merge 6 commits into
pvlib:mainfrom
cbcrespo:isotropic_components
Open

Add support for return_components to irradiance.isotropic#2787
cbcrespo wants to merge 6 commits into
pvlib:mainfrom
cbcrespo:isotropic_components

Conversation

@cbcrespo

@cbcrespo cbcrespo commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
  • Closes #xxxx
  • 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.

This PR is part of my GSoC 2026 project. One of the goals is to add support for return_components to all diffuse transposition models in pvlib, which currently is supported by some models (e.g. perez) but not others. This parameter allows the user to choose whether they want only the total diffuse irradiance (default), or are interested in the split between different diffuse components (sky diffuse, circumsolar, horizon brightening).

This has been implemented for irradiance.reindl in #2775.

For irradiance.isotropic specifically, return_components=True does not return additional information, but has been added in order to be consistent with other diffuse transposition models.

The docstrings have been changed accordingly, and a new test was added.

@kandersolar kandersolar added this to the v0.15.3 milestone Jun 18, 2026
@AdamRJensen AdamRJensen added the GSoC Contributions related to Google Summer of Code. label Jun 19, 2026

@AdamRJensen AdamRJensen 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

@AdamRJensen

Copy link
Copy Markdown
Member

Misssing a whatsnew entry fyi

@echedey-ls echedey-ls 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.

Nice PR! I want to open a discussion on the formatting of the conditional return style. Edit: as pointed out in a review comment by @cbcrespo , proposed style is already incorporated in irradiance.perez so this discussion deserves other place, if any.

Related to the previous striked-through comment

Scipy does this: https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.brentq.html#scipy.optimize.brentq

I personally would prefer the two return items, separately, with the annotation present if ... is True/False or something like that. E.g.:

    sky_diffuse : numeric (if ``return_components`` is ``False``)
        The sky diffuse component of the solar radiation. [Wm⁻²]

    diffuse_components : Dict (array input) or DataFrame (Series input) (if ``return_components`` is ``True``)
        Keys/columns are:
            * poa_sky_diffuse: Total sky diffuse
            * poa_isotropic

Comment thread pvlib/irradiance.py Outdated
Comment thread pvlib/irradiance.py
Comment thread tests/test_irradiance.py Outdated
@cbcrespo cbcrespo force-pushed the isotropic_components branch from a83746d to 97a34f6 Compare June 24, 2026 13:47
@cbcrespo

Copy link
Copy Markdown
Contributor Author

I don't understand why the docs build is failing? I checked the log and there doesn't seem to be anything related to my work.

@echedey-ls

echedey-ls commented Jun 24, 2026

Copy link
Copy Markdown
Member

I don't understand why the docs build is failing? I checked the log and there doesn't seem to be anything related to my work.

I attest that, it's not your fault @cbcrespo. I'm looking into it but it's a loophole I don't have the time to take for now. Don't worry - in the mean time you may want to try building the docs locally, it's also good to know that.

For anybody interested in debugging that @pvlib/pvlib-maintainer , these are my findings:

  1. ReadTheDocs latest release updated Ubuntu to LTS 26 https://github.com/readthedocs/readthedocs.org/releases
  2. zoneinfo python native module relies, on Linux, on tzdata package.
  3. It is reported in 2023 to RTD in Celery: ZoneInfoNotFoundError on development instance readthedocs/readthedocs.org#10453
    1. And incorporated in the PR fix Images: Add tzdata as explicit requirement readthedocs/readthedocs.org#10480
  4. Then in the following PR, when Ubuntu is updated to LTS 24, it is removed Upgrade to Ubuntu 24.04 for our application readthedocs/readthedocs.org#12002

I speculate Ubuntu LTS 26 has removed tzdata as a default package, that was introduced in the earlier Ubuntu 24 LTS (and therefore removed from their deps).

Anybody feel free to use this information, copy&paste it wherever needed. In the meanwhile, we can have an issue here in pvlib to track this issue internally, and it would also be a good idea to report it upstream.

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

Labels

enhancement GSoC Contributions related to Google Summer of Code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants