-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Input parameter naming convention - time vs times #2231
Comments
Related: #1253 |
I vote for |
I personally prefer times 😋 |
A good guiding principle may be to use plural words only when multiple values are necessary. For example, I think An argument against |
Good point. I overlooked this when agreeing with "time" with the singular/plural grammar in mind. I retract my vote; pvlib should not shadow any standard python names. |
As the shadowing only occurs within the function, this doesn't seem so serious. It will not cause any problems for the user of the function. |
Yes, but we can't anticipate how a user structures their code. |
I don't see an issue and fully agree with @adriesse on that it is not a problem. Can you provide an example of what you have in mind? AFAIK, this parameter will only override |
I can't provide an example where code fails to run. And we've had |
No problem. |
I was about to edit this function: pvlib-python/pvlib/solarposition.py Line 392 in 4cfda4a
in #2237 to add another example and test that all works flawlessly, but reading through its documentation I think the appropriate rename is Line 1106 in 29b1f86
These " Feel free to weight in if there are other words to be considered that I've missed. |
No opinion here for pvlib, but in my own work I standardized on |
That did convince me. |
Shall Line 1106 in cbe4cc5
be changed to |
Several pvlib functions take an index as one of the input parameters, however, it is inconsistent whether this parameter is called
times
ortime
.I think we should document the preferred naming, add it to the symbols and variables list, and make an effort to standardize.
See a few examples below of
TIME
:pvlib-python/pvlib/solarposition.py
Line 30 in 4cfda4a
pvlib-python/pvlib/solarposition.py
Line 128 in 4cfda4a
pvlib-python/pvlib/tools.py
Line 120 in 4cfda4a
Here's a few examples of
TIMES
:pvlib-python/pvlib/tools.py
Line 424 in 4cfda4a
pvlib-python/pvlib/solarposition.py
Line 392 in 4cfda4a
pvlib-python/pvlib/solarposition.py
Line 1343 in 4cfda4a
The text was updated successfully, but these errors were encountered: