-
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
Change Variables and Symbols page into a Glossary of terms #2234
Conversation
Irradiation is energy (e.g., Wh/m^2) whereas irradiance is power (e.g., W/m^2). I think it's better to use irradiance |
Co-Authored-By: Adam R. Jensen <39184289+adamrjensen@users.noreply.github.com>
Thoughts on retaining any of the following text from pvlib-python/docs/sphinx/source/user_guide/variables_style_rules.rst Lines 15 to 25 in cbe4cc5
|
I think its worth keeping the SodaPro references. We could drop the IEC 61724 links. |
A random thought, without any intentions: maybe instead of splitting and creating a whole new page, both of them could be put into the same one. |
If the old page is being removed, then these references need to be updated: $ git grep -n -F "variables_style_rules"
docs/sphinx/source/contributing/style_guide.rst:16::ref:`variables_style_rules`. We could be better about consistency.
docs/sphinx/source/index.rst:29:There is a :ref:`variable naming convention <variables_style_rules>` to
docs/sphinx/source/user_guide/index.rst:18: variables_style_rules
docs/sphinx/source/user_guide/variables_style_rules.rst:1:.. _variables_style_rules:
docs/sphinx/source/user_guide/variables_style_rules.rst:9: :file: ../../../../pvlib/data/variables_style_rules.csv
docs/sphinx/source/user_guide/weather_data.rst:86:into standard pvlib names (see :ref:`variables_style_rules`).
docs/sphinx/source/whatsnew/v0.3.0.txt:50: * :ref:`variables_style_rules` (:issue:`102`)
pvlib/iotools/midc.py:196: :ref:`variables_style_rules`. |
@kandersolar done, except for the 0.3.0 whatsnew entry. Anything needed there? Doesn't make sense to change the reference to "glossary", but should the :ref:`` be removed since the link won't work anyway (a warning is probably generated somewhere), or add something to say the page was removed in 11.2, or just leave it? |
The terms Nomenclature or Terminology seem to be more correct than Glossary, as the list includes both terms and symbols/variables. Glossary I believe should only contain terms/definitions. Terminology seems preferred to me as it's more easily understandable to non-native speakers. |
I don't mind "glossary", but I'm not opposed to changing the name. Just another thought: how about going back to the old name then, "Variables and Symbols"? I don't have a strong preference |
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.
Looks good overall! I left one or two comments. Regarding the name of the section, I also agree that Nomenclature (assigning of a word or phrase to a particular object, event, or property) would be a more suitable term than Glossary (terms that are either newly introduced, uncommon, or specialized)
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
I would be ok with either "Terminology" or "Variables and Symbols". |
I don't really like "Variables and Symbols"; to be precise it's more about "Common Parameters" - but I agree "Terminology" is good if it gets expanded to other terms in the future. An small side-note, the supposed link in https://pvlib-python--2234.org.readthedocs.build/en/2234/contributing/style_guide.html (first section "Code Style", second paragraph, second sentence) to the glossary is pointing to CPython's glossary: https://docs.python.org/3/glossary.html#glossary . It may be due to the crossreference at the top of the file missing an underscore at the beginning. |
@IoannisSifnaios thanks for reviewing. I was thinking it'd be better to keep the scope of this PR focused on creating the glossary. DNI was expanded as an example (definition+link in function docs). I agree the others need updating, but I think that deciding how best to define each term / groups of terms might be better addressed in a separate PR. |
Well spotted |
I created #2284 to keep track of additional suggestions and address the comments from @IoannisSifnaios and @AdamRJensen (thanks for the suggestions) Is deciding the name the only task remaining? From @IoannisSifnaios's definition, nomenclature sounds like the most technically accurate. Terminology makes me think more of words/phrases, but we have abbreviations and initialisms. That might just be my own inaccurate subjective impression though. I guess the four options now are: Nomenclature, Variables and Symbols, Glossary, Terminology How do we decide? |
I'm OK with any of these.
Sometimes the PR originator just chooses what color to paint the bikeshed. |
So much pressure 👀 |
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.
Will merge later today if no objections are raised by then
I'm still happy to make changes to any aspect if necessary, anyone please feel free to leave a comment 👍🏽 |
Thanks @RDaxini and all other participants! It's fun to see this idea finally implemented :) |
Tests addedUpdates entries indocs/sphinx/source/reference
for API changes.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`
).remote-data
) and Milestone are assigned to the Pull Request and linked Issue.The glossary would replace the variables and symbols page. This is a limited example. Links:
Glossary
Function (see parameters
surface_tilt
anddni
, and second paragraph in docstring)Points to discuss: see this comment
Note:
dni
andsurface_tilt
definition to show example enhancements (linked to the example function)Generally, I think adding/updating definitions (description, units, etc.) and adding/removing new/duplicate variables (#1421, #1253) probably requires a case-by-case discussion so I have not gone further here save for the two examples