Drop Python 2 and 3.4 #728

markcampanelli · 2019-05-17T22:29:38Z

pvlib python pull request guidelines

Thank you for your contribution to pvlib python! You may delete all of these instructions except for the list below.

You may submit a pull request with your code at any stage of completion.

The following items must be addressed before the code can be merged. Please don't hesitate to ask for help if you're unsure of how to accomplish any of the items below:

Closes issue Plan for dropping Python 2.7 #501
I am familiar with the contributing guidelines.
Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests (usually) must pass on the TravisCI and Appveyor testing services.
Updates entries to docs/sphinx/source/api.rst for API changes.
Adds description and name entries in the appropriate docs/sphinx/source/whatsnew file for all changes.
Code quality and style is sufficient. Passes LGTM and SticklerCI checks.
New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
Pull request is nearly complete and ready for detailed review.

Brief description of the problem and proposed solution (if not already fully described in the issue linked to above):

To ease development for the 0.7.0 release, let's drop support for Python 2 and 3.4 sooner rather than later.

markcampanelli · 2019-05-17T22:31:03Z

@wholmgren Mind if we do this sooner rather than later to simplify development for 0.7.0?

We can always make a 0.6.4 branch/release for any hotfixes.

wholmgren · 2019-05-17T22:34:48Z

No objection here. You'll need to drop 2.7 from the azure-pipelines.yml too.

wholmgren · 2019-05-17T22:37:23Z

another mention of python 2.7: https://pvlib-python.readthedocs.io/en/stable/contributing.html#code-style

markcampanelli · 2019-05-17T23:55:03Z

.travis.yml

    - python: 3.5
      env: CONDA_ENV=py35
    - python: 3.6
      env:
        - CONDA_ENV=py36
        - TASK="coverage"
        - DEPLOY_ENV="true"
-    - python: 3.6
+    - python: 3.7


@wholmgren Do you know if this is a significant problem in the current builds?

It is not a problem -- this only specifies the base travis python version. The conda requirements file specifies the python version that's actually used in the test environment. I figured it was better to use the same version for consistency. But Travis did not yet have a base 3.7 available when it came time to add it to build matrix, so I used 3.6.

Can you check my update in 6207754?

It's based on travis-ci/travis-ci#9815.

Also, I missed the memo on this: Is the plan to retire Travis once the Azure pipelines are deemed stable?

Can you check my update in 6207754?

I don't know what if any nuance there might be on this, but if it works and it's not noticeably slower then it's ok with me. Is there any benefit to making this change?

Is the plan to retire Travis once the Azure pipelines are deemed stable?

Main purpose of adding azure was that it let us remove appveyor. I want to see the azure pipelines documentation improve before making a plan to retire Travis.

I just set up CI using Azure pipelines for Ubuntu 16.04, Windows, and macOS here. It might be nice to maintain only a single CI in pvlib.

Thanks for the example! #719 tracks this - well, at least the macOS part. Additional work would be required to transition the deploy section of the travis config to azure. #727 also relevant.

markcampanelli · 2019-05-18T01:51:45Z

.travis.yml

@@ -63,14 +54,6 @@ install:
    - echo "install"
    - conda env create --file ci/requirements-$CONDA_ENV.yml
    - source activate test_env # all envs are named test_env in the yml files
-    # needed to make sure that pandas is compiled against the right
-    # version of numpy
-    - if [[ "$CONDA_ENV" == "py27-min" ]]; then


@wholmgren Do we need to define a new minimum environment in Python 3?

Yes, good catch. I'm pretty sure you'll be able to pin the minimum version numbers in the pip section of the file. I'm less sure about the conda section of the file. Either way is fine with me. I don't think we'll need anything special after that.

Well, I was wrong -- I'm not able to easily create a python 3.5 environment with the current numpy/pandas minimums on my mac. I suspect it's possible but I don't think it's worth the effort to struggle against theses old packages. I can easily build and test the environment with pandas 0.17:

# requirements-py35-min.yml name: test_env channels: - defaults dependencies: - python=3.5 - pip - pip: - numpy==1.10.1 - pandas==0.17.0 - pytz - nose - pytest - pytest-mock - pytest-timeout - coveralls

We should also specify a newer version of conda in the travis config. Latest is fine.

Only catch is that test_sun_rise_set_transit_spa claims it needs pandas 0.17, but it apparently needs 0.18.

markcampanelli · 2019-05-18T01:57:33Z

@wholmgren One more question: There are four different environment.yml files remaining in addition to the setup.py. It looks like the dependencies have diverged a bit between them (e.g., versions for numpy and pandas). If I can get some "official" minimums for all the required packages, then I can try to bring these files in line and test out some "clean" installs for compatibility.

wholmgren · 2019-05-18T15:32:24Z

If I can get some "official" minimums for all the required packages, then I can try to bring these files in line and test out some "clean" installs for compatibility.

Official minimums are in the setup.py file. The non-min ci/environment files intentionally don't specify versions so that we always pull the latest dependencies. The py27-min + travis.yml configuration is a workaround for some packaging issue that I don't fully recall. (Maybe conda previously iterated through the pip requirements instead specifying them simultaneously.) I think the workaround is no longer necessary and a py35-min with pinned minimum versions should be adequate. If it's not, let's address the min configuration in another issue/pr.

markcampanelli · 2019-05-19T17:57:58Z

@wholmgren I think I have all the build changes finished. I did have to update a test, which seemed to fail in strange ways in my Python 3.5 testing with the minimum numpy & pandas installations. The fix seems to make sense, but I'm not sure why it doesn't always fail.

My next (and hopefully final) step will be to remove the __future__ imports. However, can you provide some guidance on if/how I should update this? Also, can you think of any other Python 2 vs. 3 computations changes that might bite us?

wholmgren

Almost there.

I suggest that we keep this PR focused on CI and we remove the python 2 compatibility code in later PRs.

wholmgren · 2019-05-19T17:57:29Z

ci/requirements-py35.yml

-    - pandas
-    - pytz
+    - coveralls
+    - cython


why cython?

Ah, I forgot it's in the setup.py extras section. I can't remember why it's there. I guess it should be here if it's in the setup.py file.

Yes, I tried to make all the non-min requirements files match the optional setup. You'll also notice that I moved everything I could to the conda install. Anaconda recommends minimizing the mixing of conda and pip.

wholmgren · 2019-05-19T18:00:14Z

docs/environment.yml

-    - pytables
-    - pandas=0.22.0
-    - pytz
+    - coveralls


Why do you want to change the docs environment? This has been a pain to test and debug on RTD in the past. I suggest addressing separately if there's a need.

I wasn't sure what to do here, but I'm fine reverting.

Ok, please do revert this change.

wholmgren · 2019-05-19T18:01:19Z

pvlib/__init__.py

@@ -1,3 +1,10 @@
+# First ensure proper Python version.


No need for this.

I think it would help people who need to upgrade, but I'm fine removing.

wholmgren · 2019-05-19T18:03:28Z

setup.py

 EXTRAS_REQUIRE = {
    'optional': ['scipy', 'tables', 'numba', 'siphon', 'netcdf4',
-                 'ephem', 'cython', 'pvfactors'],
+                 'ephem', 'cython', 'pvfactors == 1.0.1'],


I'd rather not pin the dependency in our setup.py file.

Is this a minimum? It is pinned in the build environments?

wholmgren · 2019-05-19T18:06:27Z

setup.py

@@ -40,13 +40,12 @@
 INSTALL_REQUIRES = ['numpy >= 1.10.1',
                    'pandas >= 0.16.0',


min version change

Sorry that got reverted in some of my testing.

wholmgren · 2019-05-19T18:07:23Z

docs/sphinx/source/installation.rst

@@ -216,7 +216,7 @@ environment) when you start a new shell or terminal.
 Compatibility
 -------------

-pvlib-python is compatible with Python versions 2.7 and 3.4-3.7.
+pvlib-python is compatible with Python 3.5-7.


I suggest "3.5 and above".

Also in readme and contributing.

markcampanelli · 2019-05-19T19:17:12Z

Travis CI test statistics for 507428f:

CONDA_ENV=py35-min
- 402 passed, 201 skipped, 2 xfailed, 1 xpassed, 2 warnings in 174.61 seconds- CONDA_ENV=py35
CONDA_ENV=py35
- 604 passed, 3 skipped, 3 xfailed, 1 xpassed, 76 warnings in 244.82 seconds
CONDA_ENV=py36
- 604 passed, 3 skipped, 2 xfailed, 2 xpassed, 2 warnings in 262.36 seconds
CONDA_ENV=py37
- 605 passed, 2 skipped, 2 xfailed, 2 xpassed, 2 warnings in 132.43 seconds

markcampanelli · 2019-05-19T19:19:14Z

@wholmgren Let me know if you have any other comments. Otherwise, this should be GTG if LGTM analysis passes.

wholmgren

I'll merge tomorrow (Monday) unless there are objections.

markcampanelli · 2019-05-19T20:00:06Z

pvlib/test/test_modelchain.py

-    from unittest.mock import ANY
-except ImportError:
-    # python 2
-    from mock import ANY


Apparently, this is not used.

markcampanelli · 2019-05-19T20:29:40Z

setup.py

@@ -71,7 +67,8 @@
 setuptools_kwargs = {
    'zip_safe': False,
    'scripts': [],
-    'include_package_data': True
+    'include_package_data': True,
+    'python_requires': '>=3.5, <4'


@wholmgren I remembered that this is the more "proper" way to call this out. This way, pip install -e . won't work if the Python version is wrong.

There's an even shorter syntax for this: '~=3.5'. See https://packaging.python.org/guides/distributing-packages-using-setuptools/#python-requires

cwhanse · 2019-05-20T13:17:50Z

No objections to merge

wholmgren · 2019-05-20T21:59:35Z

Thanks @markcampanelli!

Drop Python 2 and 3.4

91e1482

markcampanelli commented May 17, 2019

View reviewed changes

markcampanelli added 2 commits May 17, 2019 18:16

Fix travis for 3.7 and update azure

6207754

Remove six dependency

5dafd6c

markcampanelli commented May 18, 2019

View reviewed changes

markcampanelli mentioned this pull request May 19, 2019

Create ivtools #718

Merged

8 tasks

markcampanelli added 2 commits May 19, 2019 11:42

Add min Travis build with consistent requirements and fixed tests

e381237

Pin pvfactors also in setup.py

ef4168d

Fix python version comparison for 3.7 patches

554c104

wholmgren reviewed May 19, 2019

View reviewed changes

markcampanelli added 4 commits May 19, 2019 12:16

Revert unneeded test change

7406bb1

Remove Python 2 mock usages

3854746

Address code review comments

990ea4e

Appease the stickler

507428f

wholmgren approved these changes May 19, 2019

View reviewed changes

markcampanelli commented May 19, 2019

View reviewed changes

Check for Python version range in setup.py

391cf72

markcampanelli commented May 19, 2019

View reviewed changes

Use more succint range syntax

11408e6

wholmgren added this to the 0.7.0 milestone May 20, 2019

wholmgren added the api label May 20, 2019

wholmgren added the installation label May 20, 2019

wholmgren merged commit d68617c into pvlib:master May 20, 2019

markcampanelli deleted the drop_python2_and_34 branch May 20, 2019 22:57

markcampanelli mentioned this pull request May 20, 2019

Remove Python 2 vestiges #729

Closed

		@@ -40,13 +40,12 @@
		INSTALL_REQUIRES = ['numpy >= 1.10.1',
		'pandas >= 0.16.0',

Drop Python 2 and 3.4 #728

Drop Python 2 and 3.4 #728

Conversation

markcampanelli commented May 17, 2019 • edited by wholmgren Loading

pvlib python pull request guidelines

markcampanelli commented May 17, 2019 • edited Loading

wholmgren commented May 17, 2019

wholmgren commented May 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markcampanelli commented May 18, 2019

wholmgren commented May 18, 2019

markcampanelli commented May 19, 2019

wholmgren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wholmgren May 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markcampanelli commented May 19, 2019

markcampanelli commented May 19, 2019

wholmgren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhanse commented May 20, 2019

wholmgren commented May 20, 2019

markcampanelli commented May 17, 2019 •

edited by wholmgren

Loading

markcampanelli commented May 17, 2019 •

edited

Loading

wholmgren May 19, 2019 •

edited

Loading