pvlib.solarposition.spa_python has unused and undocumented **kwargs

[kandersolar]

pvlib.solarposition.spa_python has **kwargs in the signature, but it doesn't mention it in the docstring, and it doesn't do anything with it in the function body.

pvlib-python/pvlib/solarposition.py

Lines 276 to 278 in c243183

	def spa_python(time, latitude, longitude,
	altitude=0, pressure=101325, temperature=12, delta_t=67.0,
	atmos_refract=None, how='numpy', numthreads=4, **kwargs):

This is bad because it silently ignores misspelled optional parameters, for example: pvlib.solarposition.spa_python(times, 40, -80, atlitude=100).

---

We could just deprecate and remove **kwargs like #1053:

pvlib-python/pvlib/location.py

Lines 83 to 87 in a6667a3

	if kwargs:
	warnings.warn(
	'Arbitrary Location kwargs are deprecated and will be '
	'removed in v0.9', pvlibDeprecationWarning
	)

Or we could keep **kwargs and pass them through to spa.solar_position(), thereby exposing the sst and esd parameters. Though if we want to expose those, I'd prefer just exposing them directly instead of hiding them in **kwargs.

This could be a good "easy first issue" once we decide what the fix should be.

[wholmgren]

I'd support removing without deprecation. Only impact on users will be to reveal bugs in their code.

[Naman-Priyadarshi]

I'll work on the issue

[cwhanse]

@kanderso-nrel it looks like the **kwargs are used to pass linke_turbidity through to spa_python from get_solarposition see the test failures for #1437

[kandersolar]

Looks like Location.get_clearsky() should be more careful about what it passes through to the solar position functions; there's no reason to pass turbidity, airmass etc through to any solar position model right? Maybe _build_kwargs is the solution here, or just using kwargs.pop() like Location.get_clearsky() does for the clearsky portion.

[cwhanse]

I agree, no reason that any solar position function needs turbidity. What do you think about handling the **kwargs in pvlib.solarposition.get_solarposition instead of in the Location methods?

[kandersolar]

What do you think about handling the **kwargs in pvlib.solarposition.get_solarposition instead of in the Location methods?

If the handling raises a TypeError when something like linke_turbidity is passed in, I don't think it avoids the problem of needing to change how Location.get_clearsky() passes kwargs around. And if it doesn't raise an error for inappropriate inputs, I think it's just shifting the problem of "silently ignoring misspelled optional parameters" to a different, and probably more commonly used, function.

On the other hand, I don't like trying to subset **kwargs in Location.get_clearsky() either since having location.py keep track of the many and various solar position parameters is not DRY.

Noting that it currently doesn't work to specify solar position kwargs in Location.get_clearsky() anyway (TypeError: ineichen() got an unexpected keyword argument 'temperature'), maybe the easiest way forward here is to just stop using **kwargs for the solar position part of Location.get_clearsky(). No functionality is lost, and people that want to customize it can still pass in their own solar position.

---

Semi-related: pvlib.irradiance.get_total_irradiance also has unused **kwargs; might as well fix that along with spa_python.

[cwhanse]

the easiest way forward here is to just stop using **kwargs for the solar position part of Location.get_clearsky()

I concur.