Make sure times are assigned with ModelChain.run_from_poa

[alorenzo175]

• Closes #xxxx
• I am familiar with the contributing guidelines
• Tests added
• Updates entries to docs/sphinx/source/api.rst 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 and Milestone are assigned to the Pull Request and linked Issue.

When running ModelChain.run_from_poa, ModelChain.times is not assigned so that the solar position results are incorrect and have a NaT index.

The issue was introduced in #1076 here

pvlib-python/pvlib/modelchain.py

Lines 1196 to 1203 in b0d29f5

	def _prep_inputs_solar_pos(self, kwargs={}):
	"""
	Assign solar position
	"""
	self.results.solar_position = self.location.get_solarposition(
	self.times, method=self.solar_position_method,
	**kwargs)
	return self

, so I'm not sure if any what's new update is needed.

[wholmgren]

looks like the same issue exists in ModelChain.run_model_from_effective_irradiance. times is not separable from weather, so maybe it should be called within _assign_weather.

[alorenzo175]

Yes it is missing there too, but I guess not a problem since it doesn't compute and assign solar position. So move it to _assign_weather?

[cwhanse]

@wholmgren it appears that the only use of ModelChain.times is as input to a solar position calculation here and here. I image those calls used weather.index before the multi-array capability. I'm not sure that ModelChain.times is consistent with the rest of ModelChain attributes, and if it persists is likely to get baked into user applications because it is convenient.

I'm OK patching this now so that pvlib behaves consistently. But down the road I think we'll want to remove ModelChain.times in favor of perhaps ModelChain.results.times?

[wholmgren]

I agree. A year ago or so I wanted to remove ModelChain.times entirely because it no longer served any purpose, but refactorings since then have brought it back to life. ModelChain.weather should also be moved to the results.

Let's move it to _assign_weather as a short term fix.

[cwhanse]

I agree. A year ago or so I wanted to remove ModelChain.times entirely because it no longer served any purpose, but refactorings since then have brought it back to life. ModelChain.weather should also be moved to the results.

Worth opening an issue so we don't forget about this