Skip to content
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

Document internal module-level constants #2096

Open
echedey-ls opened this issue Jun 17, 2024 · 5 comments
Open

Document internal module-level constants #2096

echedey-ls opened this issue Jun 17, 2024 · 5 comments

Comments

@echedey-ls
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

mikofski commented Jun 19, 2024

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

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

No branches or pull requests

4 participants