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

Bump minimum supported Python version to 3.8 #892

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

jamesobutler
Copy link
Contributor

This closes #891.

This updates the minimum supported Python version to 3.8 in similar fashion to the related numba repo PR numba/numba#8319.

@esc esc self-assigned this Nov 30, 2022
@esc esc added this to the v0.40.0 milestone Nov 30, 2022
@esc
Copy link
Member

esc commented Nov 30, 2022

@jamesobutler thank you very much!! I have queued this for a review accordingly.

Copy link
Member

@esc esc left a comment

Choose a reason for hiding this comment

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

I have reviewed this using the following patterns and was able to find any further references to Python 3.7:

buildscripts/manylinux2014/README.md:- cp37-cp37m
 💣 zsh» git --no-pager grep "3\.7"
CHANGE_LOG:This release adds support for Python 3.7 and fixes some build issues. It also
CHANGE_LOG:* PR #351: Python 3.7 compat: Properly escape repl in re.sub
CHANGE_LOG:* PR #176: Switch from LLVM 3.7 to LLVM 3.8.
CHANGE_LOG:  It was moved in LLVM 3.7.
CHANGE_LOG:* Switch from LLVM 3.6 to LLVM 3.7.  The generated IR for some memory
README.rst:0.9.0 - 0.12.1     3.7.x
docs/source/index.rst:it does not work with LLVM 3.7.0 or 3.9.0.
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<text font-family=\"Times,serif\" font-size=\"14.00\" text-anchor=\"middle\" x=\"83.7539\" y=\"-200.3\">T</text>\n",
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<path d=\"M289.496,-204.5C322.463,-204.5 305.619,-157.411 289.374,-123.761\" fill=\"none\" stroke=\"black\"/>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<text font-family=\"Times,serif\" font-size=\"14.00\" text-anchor=\"middle\" x=\"83.7539\" y=\"-200.3\">T</text>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<path d=\"M289.496,-204.5C322.463,-204.5 305.619,-157.411 289.374,-123.761\" fill=\"none\" stroke=\"black\"/>\n",
 💣 zsh» git --no-pager grep "3,7"
conda-recipes/intel-D47188-svml-VF.patch:@@ -143,6 +143,7 @@ add_llvm_component_library(LLVMAnalysis
conda-recipes/intel-D47188-svml-VF.patch:@@ -173,7 +173,7 @@ for.end:
conda-recipes/intel-D47188-svml-VF.patch:@@ -503,7 +503,7 @@ for.end:
conda-recipes/intel-D47188-svml-VF.patch:@@ -93,6 +93,7 @@ void EmitAutomata(RecordKeeper &RK, raw_ostream &OS);
conda-recipes/llvm_11_consecutive_registers.patch:@@ -9683,7 +9683,7 @@ void SelectionDAGISel::LowerArguments(const Function &F) {
 💣 zsh» git --no-pager grep "37"
CHANGE_LOG:* PR `#837 <https://github.com/numba/llvmlite/pull/837>`_: Add support for optimization remarks in pass managers (`Siu Kwan Lam <https://github.com/sklam>`_ `Andre Masella <https://github.com/apmasell>`_)
CHANGE_LOG:v0.37.0 (August 19, 2021)
CHANGE_LOG:* PR `#764 <https://github.com/numba/llvmlite/pull/764>`_: Update CHANGE_LOG for 0.37.0 (`stuartarchibald <https://github.com/stuartarchibald>`_)
CHANGE_LOG:* PR #637: Fix appveyor
CHANGE_LOG:* PR #379: ARM aarch64 test on jetson tx2
CHANGE_LOG:* PR #378: add slack, drop flowdock
CHANGE_LOG:* PR #374: Fix up broken patch selector
CHANGE_LOG:* PR #373: Add long description from readme
CHANGE_LOG:* PR #371: LLVM 6.0.1 build based on RC and fixes for PPC64LE
CHANGE_LOG:  1. A patch to fix LLVM bug (https://bugs.llvm.org/show_bug.cgi?id=37019) that
CHANGE_LOG:This is a minor release that fixes several issues (#263, #262, #258, #237) with
README.rst:As of version 0.37.0, llvmlite requires LLVM 11.x.x on all architectures
README.rst:0.37.0 - ...       11.x.x
buildscripts/manylinux2014/README.md:- cp37-cp37m
conda-recipes/0001-Revert-Limit-size-of-non-GlobalValue-name.patch:@@ -37,10 +37,6 @@
conda-recipes/appveyor/run_with_env.cmd::: http://stackoverflow.com/a/13751649/163740
conda-recipes/expect-fastmath-entrypoints-in-add-TLI-mappings.ll.patch:index c68a9c9a7..c63637294 100644
conda-recipes/intel-D47188-svml-VF.patch:index 3a7c26e..4d37b34 100644
conda-recipes/intel-D47188-svml-VF.patch:@@ -137,7 +138,9 @@ cl::opt<ActionType> Action(
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<text font-family=\"Times,serif\" font-size=\"14.00\" text-anchor=\"start\" x=\"88.7236\" y=\"-379.3\"> %out = alloca i32</text>\n",
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<text font-family=\"Times,serif\" font-size=\"14.00\" text-anchor=\"start\" x=\"88.7236\" y=\"-337.3\"> store i32 0, i32* %ct</text>\n",
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<path d=\"M83.4961,-193C83.4961,-184.558 83.4961,-175.726 83.4961,-166.937\" fill=\"none\" stroke=\"black\"/>\n",
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<path d=\"M289.496,-204.5C322.463,-204.5 305.619,-157.411 289.374,-123.761\" fill=\"none\" stroke=\"black\"/>\n",
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<text font-family=\"Times,serif\" font-size=\"14.00\" text-anchor=\"start\" x=\"51.1694\" y=\"-337.3\"> br i1 %.71, label %loop.body.preheader, label %loop.end</text>\n",
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<path d=\"M296.75,-308C296.75,-221.593 247.878,-129.175 218.466,-81.3789\" fill=\"none\" stroke=\"black\"/>\n",
docs/source/user-guide/examples/notebooks/Visualize ControlFlow.ipynb:       "<polygon fill=\"black\" points=\"221.311,-79.328 213.04,-72.7073 215.377,-83.0411 221.311,-79.328\" stroke=\"black\"/>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<text font-family=\"Times,serif\" font-size=\"14.00\" text-anchor=\"start\" x=\"88.7236\" y=\"-379.3\"> %out = alloca i32</text>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<text font-family=\"Times,serif\" font-size=\"14.00\" text-anchor=\"start\" x=\"88.7236\" y=\"-337.3\"> store i32 0, i32* %ct</text>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<path d=\"M83.4961,-193C83.4961,-184.558 83.4961,-175.726 83.4961,-166.937\" fill=\"none\" stroke=\"black\"/>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<path d=\"M289.496,-204.5C322.463,-204.5 305.619,-157.411 289.374,-123.761\" fill=\"none\" stroke=\"black\"/>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<text font-family=\"Times,serif\" font-size=\"14.00\" text-anchor=\"start\" x=\"51.1694\" y=\"-337.3\"> br i1 %.71, label %loop.body.preheader, label %loop.end</text>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<path d=\"M296.75,-308C296.75,-221.593 247.878,-129.175 218.466,-81.3789\" fill=\"none\" stroke=\"black\"/>\n",
examples/notebooks/Visualize ControlFlow.ipynb:       "<polygon fill=\"black\" points=\"221.311,-79.328 213.04,-72.7073 215.377,-83.0411 221.311,-79.328\" stroke=\"black\"/>\n",
llvmlite/tests/test_binding.py:    "5f5f617279626f002e6e6f74652e474e552d737461636b002e737472746162002e7379" \
llvmlite/tests/test_binding.py:    "6d746162003c737472696e673e00000000000000000000000000000000000000000000" \

@esc
Copy link
Member

esc commented Nov 30, 2022

I have reviewed this using the following patterns and was able to find any further references to Python 3.7:

buildscripts/manylinux2014/README.md:- cp37-cp37m

Probably that whole section will need an update to point to the available python versions.

@jamesobutler
Copy link
Contributor Author

@esc I've updated the manylinux readme to reflect the current state for llvmlite 0.39.1 where there are manylinux_2_17 wheels for Python 3.7-3.10. Then in my Bump minimum supported Python version to 3.8 commit I remove mention of the Python 3.7 manylinux_2_17 wheels.

@jamesobutler
Copy link
Contributor Author

Lucky me, in the latest push I just happened to run into one of the Ubuntu-18.04 brownout periods. I will rebase this branch against #893 to bring in the commit to use Ubuntu-20.04 so testing can continue right now.

November 30, 20:00 UTC – November 30, 22:00 UTC

@esc
Copy link
Member

esc commented Dec 1, 2022

@jamesobutler awesome thank you. I have just merged #893 -- so you can probably rebase this against main.

@jamesobutler
Copy link
Contributor Author

@esc I have rebased against main just now

@esc
Copy link
Member

esc commented Dec 1, 2022

@esc I have rebased against main just now

Thank you @jamesobutler -- I am giving this a run on the Anaconda internal Build Farm as:

Build llvmlite_178 has started

@esc
Copy link
Member

esc commented Dec 1, 2022

Looks like 3.7 is failing across the board, as expected.

Screen Shot 2022-12-01 at 14 19 25

@esc esc added 4 - Waiting on second reviewer BuildFarm Passed For PRs that have been through the buildfarm and passed and removed 3 - Ready for Review labels Dec 1, 2022
Copy link
Member

@esc esc left a comment

Choose a reason for hiding this comment

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

I have reviewed this by comparing it to previous PRs, by looking for any leftover references using grep and by running it on the Anaconda internal buildfarm. LGTM from my side, but would like to get a second set of eyes, so marking as "waiting on second reviewer".

@@ -37,7 +37,7 @@ requirements:
# llvmdev is built with libz compression support
- zlib # [unix and not (armv6l or armv7l)]
run:
- python >=3.7,<3.10
- python >=3.8,<3.10
Copy link
Member

Choose a reason for hiding this comment

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

Why is the maximum <3.10 and not <3.11? (Only tangential to this PR, I think)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed this as well and was wondering why this CI config was not doing it for Python 3.10 even though support was added back in #769.

Copy link
Member

Choose a reason for hiding this comment

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

My guess is that the conda build is ignoring / overwriting this constraint because it needs to generate one for the Python version built for anyway. For example, the run constraint from the package it builds for Python 3.10 ends up looking like python >=3.10,<3.11.0a0.

Copy link
Member

Choose a reason for hiding this comment

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

@gmarkall thank you for pointing this out.

It was introduced in: https://github.com/numba/llvmlite/pull/747/files

And not removed with:

https://github.com/numba/llvmlite/pull/769/files

As stated, I can confirm that:

tar xOf llvmlite-0.40.0dev0llvm14-py310_48.tar.bz2 info/recipe/meta.yaml

Will show:

...
  run:
    - libcxx >=4.0.1
    - python >=3.10,<3.11.0a0
    - zlib >=1.2.13,<1.3.0a0
...

So probably, this ends up not having any effect. So, in this case I would tend to lean towards removing the limit entirely, either in this PR or in a new one.

Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

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

I think this looks good - I have one question on the diff, but I don't think it's related to this PR as such.

@esc
Copy link
Member

esc commented Dec 5, 2022

@jamesobutler thank you for the patch, @gmarkall thank you for the review! We can add another PR to remove the 3.10 limit.

@jamesobutler jamesobutler deleted the drop-python-3.7 branch December 5, 2022 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Python 3.7 support removal
3 participants