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

Functions to consider making private #1756

Open
kandersolar opened this issue May 30, 2023 · 6 comments
Open

Functions to consider making private #1756

kandersolar opened this issue May 30, 2023 · 6 comments

Comments

@kandersolar
Copy link
Member

I noticed we have several functions that don't have pages in our API reference, but are also not private (name doesn't start with _):

I propose we make all these functions private (add a leading underscore to their names). I doubt they get any meaningful amount of use, so I wouldn't bother with a deprecation for any of them.

@cwhanse
Copy link
Member

cwhanse commented May 30, 2023

Only pvlib.spa.calculate_deltat has a hyperlink.

@kandersolar
Copy link
Member Author

Only pvlib.spa.calculate_deltat has a hyperlink.

Unless I'm mistaken, the calculation in calculate_deltat is not actually part of the SPA, despite living in spa.py. I wonder if it should be moved from solarposition.py. If we did that, then none of the functions in spa.py would really be user-facing (they are all supposed to be used by the wrappers in solarposition.py) and I guess the entire module could be made private (pvlib._spa).

@wholmgren
Copy link
Member

I've gone back and forth on this and ultimately I guess it's ok. It seems unnecessarily hostile to users that actually went to the effort to read source code, and we want more of those users rather than antagonizing the few that we have. I've used some of these functions in my own code, but I'm not the typical user and I can update it if I need to. And I'd like to see spa moved to its own package, but who wants to support that? Which is the point, isn't it?

@kandersolar
Copy link
Member Author

My thinking is that (with the possible exception of pvlib.tools) functions should either (1) have a stable interface and be fully documented in the sphinx docs, or (2) be an implementation detail and marked private with the underscore prefix. My motivation here isn't so much about maintenance/support burden, it's just about moving these functions into one of those two options and out of the no man's land they're in right now. So I'd be fine with bringing the SPA presence in the sphinx docs up to snuff instead of making them private like #1769 does.

I do think the non-SPA functions listed above should probably be private though.

@kandersolar
Copy link
Member Author

The SPA-related changes in #1769 are now reverted. Now it only changes the other functions listed above. I'd rather improve the SPA docs in a separate PR.

@kandersolar
Copy link
Member Author

On closer inspection, I realized scaling.latlon_to_xy was intended to be public, it just wasn't listed in the sphinx docs. #1769 now lists it instead of making it private.

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

No branches or pull requests

3 participants