Add "poa_" prefix to `return_components=True` transposition model outputs

[kandersolar]

Would the powers that be be willing to make the component name’s consistent across all model functions? For example, perez returns sky_diffuse, while poa_components returns poa_sky_diffuse. It also seems that poa_circumsolar, etc. would be more consistent and descriptive for those diffuse components. (A counter argument might be than the underlying plane need not necessarily be for a solar PV array.)

Originally posted by @markcampanelli in #1553

I think the argument makes sense. Even further: pvlib.bifacial.infinite_sheds returns keys with the poa_ prefix.

The downside is that there's not a good way to deprecate DataFrame column names (ref https://github.com/pvlib/pvlib-python/issues/1403#issuecomment-1057405773), so it would be a hard change. It's unfortunate, but I think I'm +1 to making the change.

[cwhanse]

I'm +1 to making the change

As am I. In v0.14.

[ramaroesilva]

Having this said, should #1553 / #2527 also be pushed for v0.14? And should the return_components=True bit be done in a separate PR or can we tackle both issues in one go?

[kandersolar]

The decision to wait until v0.14.0 is because we only make "breaking changes" in versions that end in .0. A breaking change is a change that could break a pvlib user's code when they update from an older version of pvlib. Changing column names to include the poa_ prefix is one example, since people currently using perez(..., return_components=True) would be using the old column names.

However, adding new (optional) functionality (for example adding return_components=False to isoptropic) isn't a breaking change, so #2527 doesn't have to wait for a .0 release.

So: adding the parameter to isotropic, klucher, etc can be done any time, while changing the returned names in perez, haydavies, etc should wait for v0.14.0.

[ramaroesilva]

Duly noted, thanks for clarifying. So, will split these two updates in separate branches/PRs.

[adriesse]

So: adding the parameter to isotropic, klucher, etc can be done any time, while changing the returned names in perez, haydavies, etc should wait for v0.14.0.

On the other hand, it could be seen as "cleaner" to bring them in together, otherwise there is a period where there is inconsistent naming of components.

[ramaroesilva]

Thanks for your comment @adriesse.

For the work done in #1553, which hopefully can be done within v0.13.1, we should keep the output names consistent as you say with the keys from diffuse_components not including the prefix for the moment.

At a later stage, for v0.14, we can address the issue of the poa_ prefix for all models together.

[kandersolar]

It sounds like there are three proposals so far:

1. Add prefixed names to isotropic et al. in v0.13.1, rename perez et al. to prefixed names in v0.14.0
2. Add prefixed names to isotropic et al. in v0.14.0, rename perez et al. to prefixed names in v0.14.0
3. Add un-prefixed names to isotropic et al. in v0.13.1, rename all functions to prefixed names in v0.14.0

Option 1 has the downside of inconsistency across functions for the period of v0.13.1–v0.14.0. Option 2 has the downside of delaying useful functionality. Option 3 has the downside of requiring additional breaking changes.

Here is a proposal that has none of those downsides:

4. Add prefixed names to isotropic et al. in v0.13.1, add (not rename) prefixed names to perez et al in v0.13.1, and drop un-prefixed names in perez et al. in v0.14.0.

The downside here is that perez output will temporarily have duplicate columns under similar names, but that seems less objectionable than the downsides of Options 1–3.

[ramaroesilva]

Nice overview! Should option 4. include a warning when using the perez and haydavies functions alerting for the expected break changes in v0.14.0? This way, old code would still work with the prefix-less calling and users would be aware and incentivized to start migrating towards the poa_ version.

[cwhanse]

+1 for Option 4. I'm not opposed to the deprecation warning @ramaroesilva suggests but it may become onerous as these transposition functions are frequently used.

[ramaroesilva]

+1 for Option 4. I'm not opposed to the deprecation warning @ramaroesilva suggests but it may become onerous as these transposition functions are frequently used.

+1 for 4 as well. Indeed, it could become a bit cumbersome for users.

Tried c633188809269b2e45a136700bcc532b704c83b3 as a first trial, without the warnings. Kept the original workflow from perez.

[adriesse]

+1 for option 1 or 2. Just for the record.

[ramaroesilva]

can anyone tell me the expected timing for v0.14 to come out (or better yet, where can such timings be found)?

knowing helps to understand how much we are delaying this functionality (in exchange of avoiding this breaking changes issue). if it's not too long, maybe I will lean towards option 2 (which would also give us time to integrate this into the get_irradiance and modelchain workflows).

@cwhanse @kandersolar @echedey-ls

[kandersolar]

can anyone tell me the expected timing for v0.14 to come out (or better yet, where can such timings be found)?

We have been doing ~quarterly releases, but that's for releases in general, so v0.14.0 doesn't have a specific date yet. If we decide the next release will be v0.14.0 (rather than v0.13.1) then it would be mid-September.

Will the next release be v0.14.0? It could be, but our last two releases were .0 releases, and I'd personally be a little reluctant for us to have three releases in a row with breaking changes. Just my 2 cents.