Skip to content
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

CI: Drop Python 3.5 support #29212

Merged
merged 18 commits into from
Nov 8, 2019
Merged

CI: Drop Python 3.5 support #29212

merged 18 commits into from
Nov 8, 2019

Conversation

datapythonista
Copy link
Member

  • closes Drop Python 3.5 support #29034
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@datapythonista datapythonista added CI Continuous Integration Python 3.5 labels Oct 24, 2019
Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

Can you update pyproject.toml and install.rst too?

Grepping for PY35 reveals a few options for cleanup. Happy to do those as followups

pandas/compat/__init__.py:15:PY35 = sys.version_info[:2] == (3, 5)
pandas/io/json/_json.py:1118:            if compat.PY35:
pandas/tests/io/json/test_json_table_schema.py:8:from pandas.compat import PY35
pandas/tests/io/json/test_json_table_schema.py:27:    if PY35:
pandas/tests/io/json/test_pandas.py:10:from pandas.compat import PY35, is_platform_32bit, is_platform_windows
pandas/tests/io/json/test_pandas.py:169:        if not numpy and PY35 and orient in ("index", "columns"):
pandas/tests/io/json/test_pandas.py:183:        if not numpy and PY35 and orient in ("index", "columns"):
pandas/tests/io/json/test_pandas.py:218:        if not numpy and PY35 and orient in ("index", "columns"):
pandas/tests/io/json/test_pandas.py:259:        if not numpy and (orient == "index" or (PY35 and orient == "columns")):
pandas/tests/io/json/test_pandas.py:326:        if not numpy and (orient == "index" or (PY35 and orient == "columns")):
pandas/tests/io/json/test_pandas.py:661:        if not numpy and PY35 and orient in ("index", "columns"):
pandas/tests/io/json/test_pandas.py:679:        if not numpy and PY35 and orient in ("index", "columns"):
pandas/tests/io/json/test_pandas.py:695:        if not numpy and PY35 and orient == "index":
pandas/tests/io/json/test_pandas.py:1619:        if PY35:
pandas/tests/frame/test_arithmetic.py:248:                r"unorderable types: .*complex\(\)",  # PY35
pandas/tests/groupby/aggregate/test_aggregate.py:576:        if compat.PY35:
pandas/tests/groupby/aggregate/test_aggregate.py:624:        if compat.PY35:
pandas/tests/scalar/period/test_period.py:13:from pandas.compat import PY35
pandas/tests/scalar/period/test_period.py:1554:    PY35,

@WillAyd
Copy link
Member

WillAyd commented Oct 24, 2019

I suggest leave it to community for follow ups as they are kind of easy to do. I can think of issues to:

  1. Rip out any Py35 compat
  2. Replace OrderedDict with just dict (maybe?)
  3. Convert annotation comments to 36 inline syntax
  4. Use f-strings instead of .format (maybe?)

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Oct 24, 2019 via email

@gfyoung
Copy link
Member

gfyoung commented Oct 24, 2019

@datapythonista : For the MacOS build (ci/deps/azure-macos-35.yaml → ci/deps/azure-macos-36.yaml), I think you need to re-specify the Python version to 3.6.*.

@datapythonista
Copy link
Member Author

Thanks for the review and the comments @TomAugspurger and @gfyoung , addressed them.

Good point @WillAyd, I included 1 here, but makes sense to create issues for 2 and 3, and not sure if we could create some for f-strings too. We don't want to get crazy with that, but in many cases that can be a nice clean up.

@@ -1,30 +0,0 @@
name: pandas-dev
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to recreate this file on 36 to pin to the lowest support versions of everything

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the info @WillAyd, didn't know that was what we were testing with that build. I restored it, moved to Python 3.6, and renamed it from compat to minimum_versions, so it's more obvious for other people what's the build about.

Copy link
Member

Choose a reason for hiding this comment

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

Cool thanks! I think that should be pinned to python 3.6.0 as well instead of allowing the latest 3.6 release to be picked up

Copy link
Member Author

Choose a reason for hiding this comment

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

makes sense, updated

@@ -573,8 +573,6 @@ def test_agg_with_one_lambda(self):

# sort for 35 and earlier
Copy link
Member

Choose a reason for hiding this comment

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

can remove

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

lgtm

- pytest-xdist
- pytest-mock
- pytest-azurepipelines
- pip
- pip:
# for python 3.5, pytest>=4.0.2, cython>=0.29.13 is not available in conda
- cython>=0.29.13
- pytest==4.5.0
- html5lib==1.0b2
Copy link
Member

Choose a reason for hiding this comment

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

Can be done as a follow up but not sure why this minimum is pinned to a beta, and I couldn't find anything in our docs for it. Probably safe to move to 1.0.1 which is available on conda

Copy link
Member Author

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

Looks like we were xfailing a test not parsing small years in Python 3.5, but the problem was an old version of dateutil. Should we require a higher dateutil because of this, or just xfail as I did here?

CC: @jreback @jbrockmendel @pganssle

# https://travis-ci.org/MacPython/pandas-wheels/jobs/574706922
PY35,
# Bug in old versions of python-dateutil (failing in 2.6.1)
StrictVersion(dateutil.__version__) <= StrictVersion("2.6.1"),
Copy link
Member Author

Choose a reason for hiding this comment

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

Failing because of dateutil, not Python 3.5

Copy link
Member

Choose a reason for hiding this comment

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

Looks like dateutil 2.7.0 was released in March 2018. I think we're safe to bump up to at least that.

Either way, really glad to see the reason for the failing test identified.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is the version breakdown of applications pinning dateutil versions. I think a disproportionate number are pinning 2.4.* because of the way fallback options for ambiguous YMD/MDY/DMY triples changed after that version.

I think it's probably fine to pin to a more recent version, but I'm not entirely sure it's warranted based on the fact that some people might experience a specific bug if they are using an old version of dateutil. I'd probably just leave the skip condition in place on the test until you have some actual new feature of dateutil that you want to use, but in the long run I'm not clear how much it will really matter.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. once the final build passes.

doc/source/getting_started/install.rst Show resolved Hide resolved
@WillAyd
Copy link
Member

WillAyd commented Nov 8, 2019

Not sure on the 36 failure here but I think @TomAugspurger cleaned up the solvability of the environment. Merge master and try again?

@datapythonista
Copy link
Member Author

Seems like Python 3.6 has something (I guess a bug) causing the error TypeError: only integer scalar arrays can be converted to a scalar index when converting strings to bytes in some of our cases.

Checked locally, and seems like tests pass again in Python 3.6.1. Updated this PR, so the minimum supported version of Python is 3.6.1.

@WillAyd
Copy link
Member

WillAyd commented Nov 8, 2019

Interesting. Not totally against it, though I wonder if we should also up our minimum numpy version as part of this.

Per NEP 29 I think they only support 1.15 with Python 3.6:

https://numpy.org/neps/nep-0029-deprecation_policy.html

So maybe worth bumping from 13.3 to that instead?

@datapythonista
Copy link
Member Author

I don't have an opinion on bumping numpy version. But I think this PR is complex enough, and dropping old numpy versions is unrelated to this. I'd discuss and implement that separately.

NEP-29 is recent. Not sure when Python 3.5 is expected to be dropped by numpy based on it, but 1.15 supports even Python 3.4.

@@ -12,7 +12,6 @@
import sys
import warnings

PY35 = sys.version_info[:2] == (3, 5)
PY36 = sys.version_info >= (3, 6)
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove this entirely in a followup I think (by-definition PY36 will always be true)

@jreback jreback added this to the 1.0 milestone Nov 8, 2019
@jreback jreback merged commit 18efcb2 into pandas-dev:master Nov 8, 2019
@jreback
Copy link
Contributor

jreback commented Nov 8, 2019

thanks @datapythonista very nice!

very small comment for possible followup.

Also would take a bump in numpy version I think we can go to min 1.15, but open an issue for it first.

@WillAyd WillAyd mentioned this pull request Nov 8, 2019
Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019
@alimcmaster1 alimcmaster1 mentioned this pull request Nov 29, 2019
2 tasks
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop Python 3.5 support
8 participants