Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
coefficient estimation method following DeSoto(2006) #784
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
coefficient estimation method following DeSoto(2006) #784
Changes from all commits
c5287ea
5a39fea
7b24201
c16f1d0
e7ef571
cdb2a73
0cfe13f
0c27829
d8653cd
bbb4580
6b8fadf
121cb06
47e1be4
c1b62c4
4e62d1a
747ed48
9ea2ed8
bd39dbd
ae5c8be
2d4f8f3
485dae6
77d88a2
6fcafc7
2d53d5d
fdbf5ec
0e1b4b3
d2d8c45
a70debb
e01f262
17f8617
e14ad40
32049b0
bfe3994
7ff8a96
7f08d80
27ab961
79ef920
5e9cf8d
c02e74f
fa41eb8
753d312
b1b405c
22d613c
ae19a68
6913a52
05051e1
d4772c6
d821494
9d23552
5037da6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other two fitting functions return tuples of values rather than a dictionary. Is there a trend one way or another?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it’s good to be consistent. However, I very much prefer interfaces where order does not matter, because this ordering is a form of complexity, which is harder to maintain/extend without breakages. Also, the dictionary output is perhaps best paired with keyword-only inputs to subsequent functions/methods, but it still works with pvlib’s current mixture of named positional and keyword arguments. However, it would not be compatible with Python 3.8’s new positional-only arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's a right answer. It would be most helpful if people could provide some good references for pros/cons of modern scientific python design patterns. I am not aware of any references that tackle non-trivial cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
24 minutes into this keynote talk on simplicity, Rich Hickey (creator of Clojure) discusses the complexity of ordering things in lists:
https://m.youtube.com/watch?v=rI8tNMsozo0&t=3s
This is not gospel of course, and indeed ordering can provide performance gains, etc. (which is why dictionaries are now ordered in CPython, which I personally treat as an implementation detail to not to be relied upon).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's address consistency among functions returns in a follow-on PR. I can't say I had a pattern in mind when I started
ivtools.py