BUG: irradiance.dirint raises KeyError with scalar inputs on pandas >= 2.0 #2751

Activity

1. cwhanse commented on May 8, 2026

   

   cwhanse

   on May 8, 2026

   Member

   More actions

   @Omesh37 thanks for the report, and we'd welcome help addressing it.

   I think the fix goes beyond #2752 and I'm not sure that casting the inputs to Series internal to dirint is the way to go.

   For starters, the docstring for dirint says the type of several inputs (the key one is solar_zenith) is array-like. Some time ago we had a long debate #765 about what array-like meant, which in pvlib means np.array or pd.Series.

   If we want to extend dirint to accept float then the type should be numeric. This is the first decision.

   If we do want float input, then I think the fix is to make irradiance._dirint_bins work with float, which may involve casting internal to that function.
2. Omesh37 commented on May 9, 2026

   

   Omesh37

   on May 9, 2026

   ContributorAuthor

   More actions

   Thanks @cwhanse for the feedback.

   • Based on discussion in use array_like instead of numeric/array-like #765, what I think is that dirint should accept numeric inputs (float/int scalars in addition to array-like), since it's a natural use case to call it with a single value.
   • My proposed updated approach:

   1. Change the type annotation for ghi, solar_zenith, pressure, and temp_dew in the dirint docstring from array-like to numeric
   2. Keep the fix inside _dirint_bins — cast scalar inputs to pd.Series using the times index so boolean masking always produces a boolean Series (not a scalar bool)
      Does that direction sound right to you? Happy to update the PR accordingly :)
3. cwhanse commented on May 9, 2026

   

   cwhanse

   on May 9, 2026

   Member

   More actions

   @Omesh37 yes, that's what I was thinking. However, please keep in mind that a pvlib function should (not must) return the same type as its inputs. In this case, if the input to dirint is float, then dirint should return dni as float. We try to do that without introducing a lot of if isinstance() kind of statement.
4. Omesh37 commented on May 10, 2026

   

   Omesh37

   on May 10, 2026

   ContributorAuthor

   More actions

   Thanks @cwhanse for conformation and guidance.
   I have updated pull request #2752.
   Please let me know if anything needs to be changed.
5. RDaxini

   added this to the v0.15.2 milestone on May 10, 2026
6. cwhanse commented on May 10, 2026

   

   cwhanse

   on May 10, 2026

   Member

   More actions

   @Omesh37 Now that I've seen what #2752 would look like, I'm reconsidering my guidance. Adding float to accepted types for dirint creates expectations that the closely-related dirindex function also accepts float (all but one of its inputs). I'm not sure we want to do all that.

   The current dirint works with arrays (which is not tested by the CI), and that's the intent of specifying array-like: array or Series.

   The float case above works correctly as long as solar_zenith is an array or Series:

   
   # works
   pvlib.irradiance.dirint(
       ghi=np.array([0,]),
       solar_zenith=np.array([90,]),
       times=pd.DatetimeIndex(['2023-06-21'], tz='UTC')
   )
   
   # also works but not by intent
   pvlib.irradiance.dirint(
       ghi=0,
       solar_zenith=np.array([90,]),
       times=pd.DatetimeIndex(['2023-06-21'], tz='UTC')
   )
   
   

   I'm inclined to stick with the array-like type, exclude floats, and ask instead that we add array to the types being tested. What do you think about the need/value of supporting float inputs?
7. Omesh37 commented on May 11, 2026

   

   Omesh37

   on May 11, 2026

   Last edited by Omesh37

   ContributorAuthor

   More actions

   Thanks for the clarification @cwhanse ! I agree on keeping array-like is cleaner. The float case is easily handled by the user wrapping in np.array([value]), and officially supporting scalars adds complexity that isn't worth it.

   Here is what I can do:

   1. Revert scalar_input check and float return from dirint
   2. Revert docstring back to array-like
   3. Revert _dirint_bins changes (not needed for proper array inputs)
   4. Update the test to use np.array inputs instead of plain int/float
   5. Add a test for checking np.array and pd.Series both work correctly

   Does it sound good?

   👍React with 👍1Reacted by Cliff Hansen
8. cwhanse commented on May 11, 2026

   

   cwhanse

   on May 11, 2026

   Member

   More actions

   @Omesh37 thanks, I agree this is cleaner.

   The new test uncovers two other discrepancies: dirint always returns Series even when no input is Series. Also, the algorithm requires 2 time points, but the code doesn't error when all inputs are length 1. Maybe that's what we want?

   I think I should talk with other maintainers before asking to change direction a third time here :)
9. Omesh37 commented on May 11, 2026

   

   Omesh37

   on May 11, 2026

   ContributorAuthor

   More actions

   Thanks @cwhanse for reviewing the code :)
   So, basically point of discussion is that:

   1. Should dirint return np.array when given np.array inputs (which will match input-type = output-type convention)?
   2. Should dirint raise a warning or error when only 1 time point is provided, since the algorithm requires 2+ points for
      delta_kt_prime?
      I'll wait for your direction for updating the PR :)
10. cwhanse commented on May 15, 2026

    

    cwhanse

    on May 15, 2026

    Member

    More actions

    @pvlib/pvlib-core two points of decision here:

    1. return type. Should dirint return np.array when np.array is input? Currently, pd.Series is returned for any valid inputs, due to some casting in a private function.

    2. should dirint raise a warning or error when inputs are time-length 1? The algorithm requires 2 time points. pvlib's code runs but returns a single NaN.
11. adriesse commented on May 15, 2026

    

    adriesse

    on May 15, 2026

    Member

    More actions

    @pvlib/pvlib-core two points of decision here:

    1. return type. Should `dirint` return `np.array` when `np.array` is input? Currently, `pd.Series` is returned for any valid inputs, due to some casting in a private function.
    

    Since a DatetimeIndex is a required input, it seems almost certain that the user is going to be working with pandas objects here anyway. There are several inputs that could be arrays or series, so what would be the logic? I would not object to returning the pd.Series to keep it simple.

    2. should `dirint` raise a warning or error when inputs are time-length 1? The algorithm requires 2 time points. pvlib's code runs but returns a single NaN.
    

    No preference. Would depend on use_delta_kt_prime=True, right?
12. Omesh37 commented on May 16, 2026

    

    Omesh37

    on May 16, 2026

    ContributorAuthor

    More actions

    Thanks for the input @adriesse!

    To confirm my understanding:

    1. Return type —> keep pd.Series always
    2. Warning —> waiting for further input on whether to add one, and whether it should depend on use_delta_kt_prime=True

    I'll wait for your instruction before making further changes :)
13. cwhanse commented on May 22, 2026

    

    cwhanse

    on May 22, 2026

    Member

    More actions

    @Omesh37 let's accept current behavior (always returns Series) and change the Returns part of docstring to state that.
14. kandersolar

    removed this from the v0.15.2 milestone on Jun 10, 2026
15. kandersolar

    closed this as not plannedon Jun 10, 2026
16. kandersolar commented on Jun 10, 2026

    

    kandersolar

    on Jun 10, 2026

    Member

    More actions

    Closing, as it seems like discussion concluded with not fixing anything with scalar inputs. The linked PR #2752 relates to test coverage for array and series inputs.

    👍React with 👍1Reacted by Echedey Luis

    ❤️React with ❤️1Reacted by Echedey Luis