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

python@3.8 3.8.0 (new formula) #47273

Closed
wants to merge 1 commit into from

Conversation

bayandin
Copy link
Member

@bayandin bayandin commented Nov 27, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

This PR adds python@3.8 formula and migrates 10 most popular depending on python formulae (but glib)

@bayandin bayandin changed the title Python 3.8.0 (new formula) python 3.8.0 (new formula) Nov 27, 2019
@bayandin bayandin mentioned this pull request Nov 27, 2019
@Bo98 Bo98 changed the title python 3.8.0 (new formula) python@3.8 3.8.0 (new formula) Nov 27, 2019
@chenrui333 chenrui333 added the new formula PR adds a new formula to Homebrew/homebrew-core label Nov 27, 2019
@Bo98
Copy link
Member

Bo98 commented Nov 27, 2019

Hmm might need to be careful and think about dependency trees here.

If a Python 3.7 formula depends on a Python 3.8 formula (or vice versa), there's gonna be problems. I'm not sure how many forumlae are in that situation.

One thing for sure is Cython. If we don't migrate everything depending on Cython together, we're gonna have to have two Cython formulae - one for Python 3.7 and one for Python 3.8. How many formulae are there depending on Cython?

@Bo98
Copy link
Member

Bo98 commented Nov 27, 2019

There's actually a lot less depending on Cython than I thought. We'll need to migrate that and their dependants (recursively) together, either now or later on.

@bayandin
Copy link
Member Author

bayandin commented Nov 27, 2019

Here it is, the list of formulae recursively depending on cython as a formula or as a resource:

adios2
apache-arrow
apache-arrow-glib
arcade-learning-environment
astrometry-net
aubio
boost-python3
breezy
btfs
caffe
cassandra
cassandra@2.2
csound
dvc
gdal
gmt
gmt@5
graph-tool
liblas
libtorrent-rasterbar
mapnik
mapserver
mpi4py
numpy
ola
opencv
opencv@3
openimageio
osm2pgrouting
pdal
pgrouting
points2grid
postgis
scipy
simple-tiles
siril
urh
virtualpg
visp
wxpython

numpy@1.16
pytouhou
urh
vapoursynth
gnuradio
gr-osmosdr
opencv@2
mpv
mvtools
vapoursynth-imwri
vapoursynth-ocr
vapoursynth-sub

@Bo98
Copy link
Member

Bo98 commented Nov 27, 2019

As a resource doesn't really matter since updating the Cython formula won't mandate updating those that use Cython as a resource at the same time. As a formula it does matter. And it needs to be done recursively (except in cases where a formula doesn't use any Python at all): brew uses --include-build --include-test --recursive cython.

You then need to watch out if anything on that list depends on another Python formula that doesn't itself depend on Cython - that too will need to be updated. Not sure of a nice way to get such list.

Cython is just an example of course - there's others too.

@bayandin
Copy link
Member Author

This PR will land python@3.8 formula only (because bumping the first 10 formulae from the list has eaten all the space on CI again)

@michaelblyons
Copy link

Can an owner add the [python] label to this PR?

@SMillerDev SMillerDev added the python Python use is a significant feature of the PR or issue label Dec 10, 2019
@SMillerDev
Copy link
Member

It doesn't really change anything but here you go

@OJFord
Copy link
Contributor

OJFord commented Dec 17, 2019

Is there anything still needed here? I understand it's just step 1 in a larger effort to get to python being at 3.8, but this PR alone is helpful for people that want to use it directly, keeping old python around for any dependencies.

@iMichka
Copy link
Member

iMichka commented Dec 17, 2019

I think we should add the caveats back (and adapt the paths in the caveat). Except that, everything looks good for me.

@bayandin
Copy link
Member Author

Thanks @iMichka, added caveats back

brew install python@2

You can install Python packages with
pip3 install <package>
Copy link
Member

Choose a reason for hiding this comment

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

As this is keg_only, I guess that the pip3 path should also be something like #{HOMEBREW_PREFIX/"bin/pip3"} ?

@iMichka
Copy link
Member

iMichka commented Dec 17, 2019

Let's merge this as-is once the build is done and green :)

@iMichka iMichka closed this in b1210f7 Dec 17, 2019
@iMichka iMichka self-assigned this Dec 17, 2019
@iMichka
Copy link
Member

iMichka commented Dec 17, 2019

I'll open another PR to fix the caveats, I noticed that some information in there is wrong.

@iMichka
Copy link
Member

iMichka commented Dec 17, 2019

Here is the PR to update the caveat: #47984

@bayandin
Copy link
Member Author

Thanks @iMichka!

@bayandin bayandin deleted the python-3.8.0-p1 branch December 17, 2019 23:31
Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

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

Good to see this move forward!!

@lock lock bot added the outdated PR was locked due to age label Jan 17, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age python Python use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants