Not a review, rather, thoughts for discussion:

• I have a preference for irradiance.complete_irradiance because that's the ModelChain method that has done a similar operation.
• Would the user experience be improved if the function returned a DataFrame with 'ghi', 'dni' and 'dhi' columns? That's closer to what the rest of pvlib would expect, and it would simplify the parameter names here which I suspect is the reason for the 'ghi_series', etc., input parameter names.

'clearsky_dni' should be described as optional.

@cwhanse on the second point, modelchains definitely prefers the dataframe with the dni, ghi, and dhi fields, but I worry about the use of a dataframe not in the context of modelchains. I set it up as separate fields for situations where the dataframe has uniquely named ghi, dni, and dhi columns. Open to either way, but that's what motivated the initial choice to make the ghi, dni, and dhi fields as separate parameters.

Is there an aversion to having e.g. 'ghi' be both an input and output parameter name?

I don't have an aversion to it if we change the name of the pvlib.irradiance.dni() function. I originally had the fields ghi, dhi, and dni as passed parameters, but it was erroring since we also have a dni() method (got confused between parameter and method). What do you think about renaming the method?

I think @cwhanse is suggesting keeping the input parameters as separate series but returning the outputs together in a single dataframe, which I agree is more consistent with other functions in pvlib.irradiance that return multiple irradiance variables. Sorry @kperrynrel I wasn't clear when we were talking before; I meant that dataframes are to be avoided as inputs to functions, but sometimes it makes sense to use them for outputs.

Something related to this if you're up for it: with this kind of function, we try to write it such that it will work equally well for scalar, numpy, and pandas inputs, and return outputs of similar type (scalar in -> scalar out, numpy in -> numpy out, pandas in -> pandas out). In this case for scalar inputs we'd return a dictionary instead of a dataframe. pvlib.irradiance.campbell_norman is a good example to follow here. One complication is that the dni function apparently doesn't follow this style and won't work as-is with scalars -- it might need a bit of rewrite to allow that if we want the new function to work with all input types. If not, ok with me to just assume pandas for now and do a follow-up PR later to add support for other types.

Regarding the dni name collision: I suggest naming the input parameter dni for consistency with the rest of pvlib and calling the dni function via pvlib.irradiance.dni(...) (this will require an import pvlib at the top of the file). It's not really ideal, but it avoids a breaking change and/or inconsistent parameter names.

Ok, @kanderso-nrel that makes more sense. Thank you guys both for the suggestions @kanderso-nrel and @cwhanse ! I'll see what I can do to adapt the code to make it fit better with the pvlib framework.

@wholmgren thank you for the review! I went ahead and worked on addressing all of your comments, and changed the PR name to the suggested name. Let me know if there's anything else I can do before merging!

thanks @kperrynrel! @kanderso-nrel can you merge if you're happy with it too?