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

Remove setuptools version pin #8475

Merged
merged 1 commit into from Oct 6, 2022
Merged

Conversation

gmarkall
Copy link
Member

No description provided.

@esc
Copy link
Member

esc commented Sep 30, 2022

@gmarkall is this a re-roll of #8366 and #8474 ? Do we want to use this one instead and close #8366 ?

@gmarkall
Copy link
Member Author

@gmarkall is this a re-roll of #8366 and #8474 ? Do we want to use this one instead and close #8366 ?

No, it is still a dependency but not on any particular version. This is just a quick fix for the issues it causes (with potentially some new test issues introduced in tests for warnings, that may show up on the buildfarm).

@gmarkall
Copy link
Member Author

Marking is RFR to see what people (and potentially the buildfarm run) think.

@stuartarchibald
Copy link
Contributor

Thanks for the patch @gmarkall, as discussed OOB and in various issues, this appears to be an approximate revert of #8356 which makes the version of the setuptools runtime dependency unrestricted. On inspection only the patch looks good.

I was considering that this test might fail:

def test_no_accidental_warnings(self):
# checks that importing Numba isn't accidentally triggering warnings due
# to e.g. deprecated use of import locations from Python's stdlib
code = "import numba"
# See: https://github.com/numba/numba/issues/6831
# bug in setuptools/packaging causing a deprecation warning
flags = ["-Werror", "-Wignore::DeprecationWarning:packaging.version:"]
run_in_subprocess(code, flags)

but on second look, I don't think it will as importing numba doesn't import numba.pycc and so the numpy.distutils module won't be accessed.

@esc
Copy link
Member

esc commented Sep 30, 2022

Buildfarm ID: numba_smoketest_cpu_yaml_135

@sklam
Copy link
Member

sklam commented Sep 30, 2022

I tested it on my mac and on win64 with setuptools=65 and numpy=1.23.

@gmarkall
Copy link
Member Author

gmarkall commented Oct 3, 2022

How did the buildfarm run go @esc?

@gmarkall gmarkall added this to the Numba 0.56.3RC milestone Oct 3, 2022
@gmarkall
Copy link
Member Author

gmarkall commented Oct 3, 2022

I've put this in the 0.56.3RC milestone for now - I think this is the change we intend to consider for it (please let me know if this understanding was not right).

@gmarkall gmarkall added the Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm label Oct 3, 2022
@esc
Copy link
Member

esc commented Oct 4, 2022

How did the buildfarm run go @esc?

There were some issues that @sklam is looking into.

@stuartarchibald stuartarchibald self-assigned this Oct 4, 2022
@stuartarchibald stuartarchibald added 4 - Waiting on reviewer Waiting for reviewer to respond to author 4 - Waiting on CI Review etc done, waiting for CI to finish and removed 3 - Ready for Review labels Oct 4, 2022
@gmarkall
Copy link
Member Author

gmarkall commented Oct 4, 2022

How did the buildfarm run go @esc?

There were some issues that @sklam is looking into.

Just to check, these are expected to be problems with the buildfarm and not this PR?

@esc
Copy link
Member

esc commented Oct 4, 2022

How did the buildfarm run go @esc?

There were some issues that @sklam is looking into.

Just to check, these are expected to be problems with the buildfarm and not this PR?

This seems likely, yes, but isn't 100% conclusive, yet.

Copy link
Member

@sklam sklam left a comment

Choose a reason for hiding this comment

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

Ok on the farm

@sklam sklam added 5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed and removed Pending BuildFarm For PRs that have been reviewed but pending a push through our buildfarm 4 - Waiting on CI Review etc done, waiting for CI to finish 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Oct 6, 2022
@sklam sklam merged commit 1c07efb into numba:main Oct 6, 2022
stuartarchibald pushed a commit to stuartarchibald/numba that referenced this pull request Oct 12, 2022
Remove setuptools version pin

Resolved conflicts in:
	buildscripts/condarecipe.local/meta.yaml
	buildscripts/incremental/setup_conda_environment.sh
	setup.py
@mbargull
Copy link
Contributor

It probably makes sense to restrict the build time dependency (runtime dependency might be fine to be unrestricted) given that setuptools can break numpy.distutils as happened again with the latest setuptools=65.6.0 release and NumPy themselves pin to setuptools<60.0.
See numpy/numpy#22623 (comment) and issues linked therein.

@stuartarchibald
Copy link
Contributor

It probably makes sense to restrict the build time dependency (runtime dependency might be fine to be unrestricted) given that setuptools can break numpy.distutils as happened again with the latest setuptools=65.6.0 release and NumPy themselves pin to setuptools<60.0. See numpy/numpy#22623 (comment) and issues linked therein.

Thanks for raising this @mbargull, I've opened #8617 to ensure that this is discussed by the maintainers at the next triage meeting. There are a few patches pending review for the next Numba release that are hopefully going to remove a lot of the reliance on setuptools and numpy.distutils which might decrease the exposure to upstream changes. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to merge Review and testing done, is ready to merge BuildFarm Passed For PRs that have been through the buildfarm and passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants