Skip to content

move ModelChain.times, weather to ModelChainResult #1164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wholmgren opened this issue Feb 5, 2021 · 0 comments · Fixed by #1197
Closed

move ModelChain.times, weather to ModelChainResult #1164

wholmgren opened this issue Feb 5, 2021 · 0 comments · Fixed by #1197
Labels
Milestone

Comments

@wholmgren
Copy link
Member

ModelChain.__init__ sets attributes times and weather to None. The ModelChain.run_* methods store the input weather data on weather and they store the index of the input weather data on times.

The weather data may be a tuple of DataFrames, so times provides convenient access to the common index (other functions enforce the common index requirement) for computing solar position. ModelChain.times is not otherwise used.

I think these attributes should move to ModelChainResults. While it's fair to question if the input weather can be called "results", the bigger picture is that moving the weather/times data to the ModelChainResults object puts all of the mutable data in a single place. The remaining ModelChain attributes would be nominally immutable model specifications. I think that's worth the slight inconsistency in naming.

Discussion started in #1162.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant