Using a dict here allows the function call to be double-splatted (**input) and may make it easier to "see" each tested set of inputs and output. It's not the style used for other tests are parameterized.

I'm interested in feedback from others: is this parameterization style easier or more difficult, as a reviewer?

My 2 cents: a more compact representation of the changing values is helpful for reviewing, and is worth a bit of processing inside the test function itself. For example, it seems like the following:

(
    {
        "time": pd.DatetimeIndex(
            ["1974-06-22T18:30:15"],
            tz=ZoneInfo("Etc/GMT+5"),
        ),
        "location": location.Location(
            43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
        )
    },
    pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),
(
    {
        "time": pd.DatetimeIndex(["1974-06-22T18:30:15"]),
        "location": location.Location(
            43.19262774396091, -77.58782907414867, tz="Etc/GMT+5"
        )
    },
    pd.DatetimeIndex(["1974-06-22T23:30:15"], tz=ZoneInfo("UTC")),
),

could be replaced with something like this:

("1974-06-22T18:30:15", "Etc/GMT+5", 43, -77, "Etc/GMT+5", "1974-06-22T23:30:15+00:00"),
("1974-06-22T18:30:15", None, 43, -77, "Etc/GMT+5", "1974-06-22T23:30:15+00:00"),

And the test function could then contain the necessary calls to Location and pd.DatetimeIndex to construct the parameters themselves.

I think the latter format makes it easier to see what changes between each set of parameters. I found my eyes having to flick back and forth and subsequently getting lost with the former format.

In the end I think this was simplified to

@pytest.mark.parametrize(
    'tz,tz_expected', [
        pytest.param('UTC', 'UTC'),
        pytest.param('Etc/GMT+5', 'Etc/GMT+5'),
        pytest.param('US/Mountain', 'US/Mountain'),
        pytest.param('America/Phoenix', 'America/Phoenix'),
        pytest.param('Asia/Kathmandu', 'Asia/Kathmandu'),
        pytest.param('Asia/Yangon', 'Asia/Yangon'),
        pytest.param(datetime.timezone.utc, 'UTC'),
        pytest.param(zoneinfo.ZoneInfo('Etc/GMT-7'), 'Etc/GMT-7'),
        pytest.param(pytz.timezone('US/Arizona'), 'US/Arizona'),
        pytest.param(-6, 'Etc/GMT+6'),
        pytest.param(-11.0, 'Etc/GMT+11'),
        pytest.param(12, 'Etc/GMT-12'),
    ],
)
...

I'm inclined to ask to revert this change. The docstring says tz can be a float. There are places that have a non-integer offset from UTC, e.g., the Marquesas are UTC−09:30UTC−09:30. localize_to_utc is changed in this PR to examine Location.pytz rather than Location.tz, so the error we discussed won't occur.

There is a point in the Location docstring that says this:

Location objects have two timezone attributes:

    * ``tz`` is a IANA timezone string.
    * ``pytz`` is a pytz timezone object.

While I appreciate my changes may break some things, I think this existing docstring suggests a much more sane approach. (Indeed, in the sequel using zoneinfo, I propose using one, and only one, internal representation of the timezone, instead of two, potentially with helper functions s to update this internal representation using int/float offsets, etc..)

I'm not saying revert from storing a string, but revert the int portion. If tz is truncated to an integer, how are timezones such as UTC+05:30 represented?

I'm open to doing away with accepting int or float for tz.

@cwhanse Sorry I misunderstood your meaning. I was aware that some locations are on half-hour offsets and such, but what is unclear to me is if/how that is represented in pytz (or zoneinfo for that matter). I do not see any fractional offsets in zoneinfo.available_timezones() and this also does not work for me: pytz.timezone("Etc/GMT+5.5").

I must be missing something here. Does something non-standard have to be done to support fractional offsets?

I'm open to doing away with accepting int or float for tz.

Hi again @cwhanse. It looks like the standard "non-integral-hour" offsets are only supported in the IANA standard through place names such as "Asia/Kathmandu" and "Asia/Yangon", but not through Etc/GMT+X.Y style offsets.

Given that, I think the most trouble-free thing to do is to remove the int and float options and be sure to discuss this issue in the documentation tutorial. (Also, the only supported tz strings would be the IANA ones.) Sounds like you are amenable to this.

Keeping both fields tz (only as a string) and pytz (as a pytz timezone type), then I think I should add setters that change one when the other is changed after the Location object has already been already initialized.

I think the most trouble-free thing to do is to remove the int and float options

I'm in favor of this change. @pvlib/pvlib-maintainer @pvlib/pvlib-triage your opinions?

No opinion.

TMY3 files represent timezone in numeric form, so this change may complicate some workflows. Outside of that, I never use this attribute (and don't know why I would), so I have no opinion either.

Remember that we usually have a deprecation period before outright removal.

I did some more investigating on this yesterday, and I think I see an implementation that would better support the existing int and float offsets. However, this also raised some new questions about the existing implementation. I will try to push some concrete code here by end of day for people to consider, along with the new questions raised.

Ok. I have added support back for int and non-fractional float being passed to tz in the Location constructor. I also test that pytz and zoneinfo timezones are likewise supported for initialization, and I beefed up the tests even more.

Note that the pytz Location attribute is now the single source of truth for the time zone in a Location object (which I recommend updating to a standard-library timezone.TimeZone object in #2342). self.tz simply returns the stringified self.pytz, but one can still directly set self.tz with any tz value supported by the Location intializer. Also, the tz and pytz attributes are kept in sync now, as changing one updates the other.

Ready for another review.

Ensuring types is a bug fix. The remainder describes an API change that should be announced elsewhere. Not really an enhancement, so maybe we put the rest of this note in the "Breaking changes" section, because one can no longer set a value for .pytz.

@cwhanse Lmk if my whatsnew change for this looks ok.

I think the focus of this note should be on how the changes will affect user code.