Skip to content

Bump minimum pandas version to 0.22.0, switch from pd.util.testing to pd.testing #1003

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 19, 2020
Merged

Bump minimum pandas version to 0.22.0, switch from pd.util.testing to pd.testing #1003

merged 5 commits into from
Jul 19, 2020

Conversation

kandersolar
Copy link
Member

@kandersolar kandersolar commented Jul 15, 2020 •
edited by cwhanse
Loading

  • Closes switch from pandas.util.testing to pandas.testing #901
  • I am familiar with the contributing guidelines
  • Tests added
  • Updates entries to docs/sphinx/source/api.rst for API changes.
  • Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
  • New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

Only the assert_* functions are exposed in the public module pandas.testing, so I had to import the network decorator from pandas._testing instead. I'm not sure why it isn't public, but its docstring example imports from _testing so I guess that's the way to do it.

git grep -n 'util\.test' only shows one remaining instance, an output cell in the tracking tutorial notebook that should be taken care of by #999.

FYI there is a new deprecation warning:

pvlib/iotools/ecmwf_macc.py:303: DeprecationWarning: tostring() is deprecated. Use tobytes() instead.
    times = netCDF4.num2date(nctime[time_slice], nctime.units)

Looks like it only gets raised in our tests from netCDF4 calls. See Unidata/netcdf4-python#1023

@kandersolar
Copy link
Member Author

The python 3.5 checks are failing, I suppose because the latest pandas version available for 3.5 doesn't have pandas._testing for the network decorator. Some ways forward:

  • just stop using the network decorator. Not sure what value it provides now anyway, unless people like to run the tests while not connected to the internet?
  • drop support for 3.5 and require pandas >= 1.0.0
  • change where network is imported from based on the pandas version
  • hold off on merging this

@wholmgren
Copy link
Member

just stop using the network decorator. Not sure what value it provides now anyway, unless people like to run the tests while not connected to the internet?

my preference

@cwhanse cwhanse added this to the 0.8.0 milestone Jul 15, 2020
@wholmgren
Copy link
Member

It appears that none of the test failures are due to pandas min dependency not being increased, and it appears that we've lost the test configuration that used https://github.com/pvlib/pvlib-python/blob/master/ci/requirements-py35-min.yml

I think we need to get that working again before we can take action on this PR.

@kandersolar
Copy link
Member Author

it appears that we've lost the test configuration that used https://github.com/pvlib/pvlib-python/blob/master/ci/requirements-py35-min.yml

Do I understand correctly that py35-min is used on Travis but not Azure? It seems like Travis is still running those tests, despite not posting the results to the PR: https://travis-ci.org/github/pvlib/pvlib-python/builds/708201652. This same thing happened with RdTools too: NREL/rdtools#138 (comment) -- maybe the travis yaml is out of date or something.

@kandersolar
Copy link
Member Author

Possibly relevant, but I think only someone with admin access to pvlib can do this: https://travis-ci.community/t/github-status-not-posted-on-commits-on-repositories-using-legacy-service-integration/7798

@wholmgren wholmgren mentioned this pull request Jul 17, 2020
Merged
8 tasks
@kandersolar
Copy link
Member Author

After merging master so that python35-min is included, we do get failures on pandas 0.18.1: https://dev.azure.com/solararbiter/pvlib%20python/_build/results?buildId=3817&view=logs&j=b0b9a916-ca36-5efa-d29b-53c06cc9f57e&t=4c652da1-4a81-5d6f-5221-c0e955e0fbca&l=31

Should we bump pandas to 0.22 in this PR?

@wholmgren
Copy link
Member

wholmgren commented Jul 18, 2020 via email

wholmgren
wholmgren approved these changes Jul 18, 2020
View reviewed changes
Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor doc suggestion below. Test failures are unrelated, so we can go ahead and merge.

@kandersolar kandersolar changed the title Switch from pd.util.testing to pd.testing and pd._testing Bump minimum pandas version to 0.22.0, switch from pd.util.testing to pd.testing Jul 18, 2020
@wholmgren wholmgren merged commit 010a2ad into pvlib:master Jul 19, 2020
@kandersolar kandersolar deleted the pd_testing branch July 19, 2020 18:38
@kandersolar kandersolar mentioned this pull request Jul 22, 2020
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wholmgren wholmgren wholmgren approved these changes

Assignees
No one assigned
Labels
testing
Projects
None yet
Milestone
0.8.0
Development

Successfully merging this pull request may close these issues.

switch from pandas.util.testing to pandas.testing
3 participants
@kandersolar @wholmgren @cwhanse