pvlib v1.0 proposal & branching / benchmarking strategy for reduced complexity and better maintainability

[PhilBrk8]

So, I know v.1.0 is a huge proposal that would result in a lot of work for a lot of people - sorry in advance -
but please hear me out on how I got there:

I initially and naively wanted to see pvlib adopt polars for some performance gains, which would result in many breaking changes, so I dropped the idea because it seemed unrealistic.
But I am still 100% backing up the argument of 1926 for a transition from pandas to polars.

A while later I returned to the idea of working through the repo and re-run the command pip install pvlib -e with the goal in mind to implement type hints, which I have been proposed several times now. for example in #2062

Since I would then go through every function and tough each file with this anyways, I ended up at the idea of bringing the code up to +3.10 syntax in general and reduce some code complexity and remove some legacy code on the way while implementing a more modern way to handle things.

---

If my goal is to reduce code complexity and improve maintainability, then I would drop support for legacy code and for example not provide input possibilities for pandas and polars timeseries for example.
And I believe the library is old and grown enough that its time to clean up a bit, focus on performance and maintainability,
and allow to ...

---

BUT - jokes aside:
Obviously this should not be done or taken lightly, so my overarching proposal / question is the following:
Is the community / are the maintainers in favour of setting up a branching strategy in this repo, with at least a development or maybe some form of v.1.0 branch?

I believe this might be a good way to benchmark the v.0xxx and v.1xxx performance against each other somewhere in the future.

---

here an example of how code could look like with applied changes and ruff formatter:
(compare to the solarposition.py file yourself if you want to check it out)

import datetime as dt
import os
import warnings
from importlib import reload
from typing import Any, Literal

import ephem
import numpy as np
import pandas as pd
import polars as pl
import scipy.optimize as so
from pydantic_extra_types.coordinate import Coordinate, Latitude, Longitude,

# Coordinate = Tuple[Latitude, Longitude]
from pvlib import atmosphere, tools
from pvlib.tools import datetime_to_djd, djd_to_datetime


def ensure_timeseries_input_format(
    timeseries: pl.Series, expected_unit: Literal["ms", "us", "ns"] = "ms"
) -> pl.Series:
    """Ensures that timeseries is a Polars Datetime Series that has a timezone"""

    # 1. Must be a Datetime Series
    if not isinstance(timeseries.dtype, pl.datatypes.Datetime):
        raise TypeError(f"Expected a Datetime Series, got {timeseries.dtype}")

    # 2. If naive, treat it as UTC
    if timeseries.dtype.time_zone is None:
        timeseries = timeseries.dt.replace_time_zone("UTC")

    # 3. After replace_time_zone, if still no tz, raise an error
    if timeseries.dtype.time_zone is None:
        raise ValueError("Datetime Series is still naive (no timezone set).")

    # 4. Ensure correct time unit
    if timeseries.dtype.time_unit != expected_unit:
        raise TypeError(
            f"Expected Datetime('{expected_unit}', tz={timeseries.dtype.time_zone}), got {timeseries.dtype}"
        )

    timeseries_utc = timeseries.dt.convert_time_zone("UTC")
    unix_time_arr = timeseries_utc.dt.epoch(time_unit="s").to_numpy()

    return timeseries, timeseries_utc, unix_time_arr


def get_solarposition(
    timeseries: pl.Series,
    coordinates: Coordinate,
    ######## or, possibly better in order to prevent too many unnecessary changes:
    latitude: Latitude,
    longitude: Longitude,
    ########
    altitude: float | None = None,
    pressure: float | None = None,
    method: Literal[
        "nrel_c", "nrel_numba", "nrel_numpy", "pyephem", "ephemeris"
    ] = "nrel_numpy",
    temperature: float = 12.0,
    **kwargs: Any,
) -> pl.DataFrame:
    """
    ...docstrings...
    """

    if altitude is None and pressure is None:
        altitude = 0.0
        pressure = 101325.0
    elif altitude is None:
        altitude = atmosphere.pres2alt(pressure)
    elif pressure is None:
        pressure = atmosphere.alt2pres(altitude)

    timeseries, timeseries_utc, unix_time_arr = ensure_timeseries_input_format(timeseries)

    match method.lower():
        case "nrel_c":
            ephem_df = spa_c(
                timeseries,
                latitude,
                longitude,
                pressure,
                temperature,
                **kwargs,
            )
        case "nrel_numba":
            ephem_df = spa_python(
                timeseries,
                timeseries_utc,
                unix_time_arr,
                latitude,
                longitude,
                altitude,
                pressure,
                temperature,
                how="numba",
                **kwargs,
            )
        case "nrel_numpy":
            ephem_df = spa_python(
                timeseries,
                timeseries_utc,
                unix_time_arr,
                latitude,
                longitude,
                altitude,
                pressure,
                temperature,
                how="numpy",
                **kwargs,
            )
        case "pyephem":
            ephem_df = pyephem(
                timeseries,
                latitude,
                longitude,
                altitude=altitude,
                pressure=pressure,
                temperature=temperature,
                **kwargs,
            )
        case "ephemeris":
            ephem_df = ephemeris(
                timeseries,
                latitude,
                longitude,
                pressure,
                temperature,
                **kwargs,
            )
        case _:
            raise ValueError(f"Invalid solar position method: {method}")

    return ephem_df

[markcampanelli]

@PhilBrk8 The Limp Bizkit video made my morning. Thank you!

Maybe propose refactoring pandas to be higher up in/removed from the pvlib “stack” before taking on typing?

I type all my Python code now. However, I’ve largely given up this crusade for pvlib because the documentation is pretty damn good already and there’s no computational performance benefit for doing all the extra work.