Skip to content

Various pandas deprecation warnings from test suite #1892

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

Closed
kandersolar opened this issue Oct 17, 2023 · 8 comments · Fixed by #1900
Closed

Various pandas deprecation warnings from test suite #1892

kandersolar opened this issue Oct 17, 2023 · 8 comments · Fixed by #1900

Comments

@kandersolar
Copy link
Member

We've started getting several types of deprecation warnings from the tests:

pvlib/tests/test_modelchain.py::test_aoi_model_interp
  /home/runner/work/pvlib-python/pvlib-python/pvlib/tests/test_modelchain.py:1476: FutureWarning: Series.__getitem__ treating keys as positions is deprecated. In a future version, integer keys will always be treated as labels (consistent with DataFrame behavior). To access a value by position, use `ser.iloc[pos]`
    assert mc.results.ac[1] < 1
pvlib/tests/test_clearsky.py::test_detect_clearsky_window
  /home/runner/work/pvlib-python/pvlib-python/pvlib/tests/test_clearsky.py:634: FutureWarning: Setting an item of incompatible dtype is deprecated and will raise in a future error of pandas. Value 'True' has dtype incompatible with int64, please explicitly cast to a compatible dtype first.
    expected.iloc[-3:] = True
pvlib/tests/test_singlediode.py: 14 warnings
  /home/runner/work/pvlib-python/pvlib-python/pvlib/tests/test_singlediode.py:118: FutureWarning: DataFrame.applymap has been deprecated. Use DataFrame.map instead.
    joined[is_number] = joined[is_number].applymap(np.float64)
  /home/runner/work/pvlib-python/pvlib-python/pvlib/scaling.py:279: FutureWarning: DataFrame.fillna with 'method' is deprecated and will raise in a future version. Use obj.ffill() or obj.bfill() instead.
    df = df.fillna(method='bfill').fillna(method='ffill')

Here's an example run: https://github.com/pvlib/pvlib-python/actions/runs/6550206298/job/17788805508?pr=1827#step:9:65

Addressing these would be good first PRs for anyone wanting to get started with contributing to pvlib's codebase! If you plan to submit a PR for one of these warnings, please comment here so that someone else does not duplicate the work.

@matsuobasho
Copy link
Contributor

Is the intention to resolve these warnings by updating the deprecated methods with current ones, or rather to just hide the deprecation warnings?

@cwhanse
Copy link
Member

cwhanse commented Oct 24, 2023

@matsuobasho preferably update the methods to resolve the warnings. If updating breaks other code, perhaps hide them.

@matsuobasho
Copy link
Contributor

matsuobasho commented Oct 28, 2023

Ok, I'm tackling this issue. Since is this my first issue, would like to ask about a nuance that I've encountered.
In the test_bird function of test_clearsky.py, we get a warning on lines 857-859:

irrads3 = clearsky.bird(
        zenith[12], airmass[12], aod_380nm, aod_500nm, h2o_cm, dni_extra=etr[12]
    )

Here's the warning:

FutureWarning: Series.__getitem__ treating keys as positions is deprecated. In a future version, integer keys will always be treated as labels (consistent with DataFrame behavior). To access a value by position, use `ser.iloc[pos]`

The interesting part is that the warning only pertains to etr[12]. The reason why is that out of the 3 objects that are being referenced by an index, only etr is a Pandas series. zenith is a Pandas index type, while airmass is a numpy array. Neither of those have an iloc attribute.

So changing to etr.iloc[12] gets rid of the warning. However, there is now an inconsistency in how we would be referencing these objects. I think it's fine but looking for confirmation especially as I'll potentially run into the same scenario in other warnings that are part of this issue. Thanks.

@kandersolar
Copy link
Member Author

Wow, three different vector types going into a single function :P Don't worry about the inconsistency with not using iloc for all inputs in that case; changing to just etr.iloc[12] is fine. Maybe that test could be cleaned up a bit more generally, but that would be a separate PR. Thanks @matsuobasho!

@matsuobasho
Copy link
Contributor

matsuobasho commented Oct 30, 2023

I made the updates to get rid of the errors but I get a bunch of failures when I run pytest and I can't be sure that the warnings in question don't also appear in the functions that fail. For example:

image

Please recommend a course of action, thanks.

@kandersolar
Copy link
Member Author

Ah yeah, the SRML website has been down for a while and causing the associated tests to fail. If you check the recent CI test results for main you'll see the same failures there.

I suggest ignoring those failing tests for now. If any deprecation warnings come out of those tests whenever they eventually start passing again, we can just address them at that time.

@matsuobasho
Copy link
Contributor

matsuobasho commented Oct 30, 2023

  1. To double-check, are the warnings below in-scope for this issue as well?
    image

  2. The pull request submission form has the following:

  • Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.
    Do I need to add any additional labels to this PR / issue?

@cwhanse
Copy link
Member

cwhanse commented Oct 30, 2023

@matsuobasho we add the labels to the PR.

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

Successfully merging a pull request may close this issue.

3 participants