search

pvlib / pvlib-python

A set of documented functions for simulating the performance of photovoltaic energy systems.
https://pvlib-python.readthedocs.io
BSD 3-Clause "New" or "Revised" License
1.17k stars 993 forks source link

Document internal module-level constants #2096

Open echedey-ls opened 3 months ago

echedey-ls commented 3 months ago

Is your feature request related to a problem? Please describe. A lot of times constants are referenced in the documentation, but the only way to access them is either by visual inspection of the code or importing the object that contains it programmatically.

Describe the solution you'd like Include constants in a toctree and add their respective documentation.

Describe alternatives you've considered A toctree dedicated exclusively to all pvlib constants.

Additional context https://stackoverflow.com/questions/40748886/how-can-i-document-a-constant-module-level-variable-with-sphinx-docstring-wit https://stackoverflow.com/questions/50831070/how-to-document-linkable-constants-with-sphinx

kandersolar commented 3 months ago

To my knowledge, the current sphinx docs document only one module-level variable; see the bottom of this page: https://pvlib-python.readthedocs.io/en/stable/reference/pv_modeling/temperature.html

I think documenting the variables near the functions they are related to makes sense. I don't think a toctree for all variables makes sense.

AdamRJensen commented 3 months ago

Agree with Kevin that they should be documented near where they belong. I am overall very supportive of documenting them! For example, the SURFACE_ALBEDOS comes from a publication and that would be nice to know.

mikofski commented 3 months ago

Should we consider properties for constants. Although highly unlikely, constants could be accidentally overwritten and currently nothing in pvlib would alert you to this. Although clumsy a property is read only and cannot be changed without a set method

echedey-ls commented 3 months ago

Although highly unlikely, constants could be accidentally overwritten and currently nothing in pvlib would alert you to this

I'm only in favour about marking their type as Final. It would alert linters. The same way we almost never check input values to models because we expect the user-base to know what they are doing, I would not try to over-complicate providing some handy values.

mikofski commented 3 months ago

Agreed, I only mention it here for posterity in case this becomes an issue again in the future. The documentation of TEMPERATURE was previously an issue, & I had suggested making it a class which allows it & its members to be documented. This was a drop in replacement, although admittedly unnecessarily overcomplicated