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

Jump to bottom

FIX Don't use setuptools 58.5 which introduces regression #21549

Merged
merged 3 commits into from Nov 4, 2021
Merged

FIX Don't use setuptools 58.5 which introduces regression #21549

merged 3 commits into from Nov 4, 2021
+21 −7

Conversation

Reviewers

@ogrisel ogrisel

@adrinjalali adrinjalali

@glemaitre glemaitre

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@jjerphan @ogrisel @glemaitre @adrinjalali
@jjerphan
Copy link
Member

@jjerphan jjerphan commented Nov 4, 2021

Reference Issues/PRs

pypa/setuptools#2849

What does this implement/fix? Explain your changes.

This is a temporary fix for: pypa/setuptools#2849
@jjerphan
FIX Don't use setuptools 58.5 which introduces regression
59ed703 
This is a temporary fix for:
pypa/setuptools#2849
@jjerphan jjerphan requested a review from glemaitre Nov 4, 2021
@jjerphan jjerphan marked this pull request as draft Nov 4, 2021
ogrisel
ogrisel reviewed Nov 4, 2021
View changes
pyproject.toml Show resolved Hide resolved
Loading
@ogrisel
Copy link
Member

@ogrisel ogrisel commented Nov 4, 2021 •
edited
Loading

Unfortunately changing the dependency version in pyproject.toml alone does not work, probably because we do not use build isolation on the CI. I think we also need to change the setuptools setup in the configuration of the CI jobs.

Loading

@jjerphan
[WIP] Adapt CI for workaround
c6d1767
ogrisel
ogrisel approved these changes Nov 4, 2021
View changes
Copy link
Member

@ogrisel ogrisel left a comment

LGTM assuming this fixes the CI.

Loading

@jjerphan jjerphan marked this pull request as ready for review Nov 4, 2021
@jjerphan
[WIP] Adapt CI for workaround
b152587
@ogrisel
Copy link
Member

@ogrisel ogrisel commented Nov 4, 2021

It seems to work on most builds. Let's wait for the fast tests to complete and merge without waiting for the slow PyPy job.

Loading

adrinjalali
adrinjalali approved these changes Nov 4, 2021
View changes
Copy link
Member

@adrinjalali adrinjalali left a comment

Somebody needs to remember to revert this PR once the issue is fixed on the setuptools side 😁

Loading

@jjerphan
Copy link
Member Author

@jjerphan jjerphan commented Nov 4, 2021

Somebody needs to remember to revert this PR once the issue is fixed on the setuptools side grin

I propose to be responsible for reverting the commit when the issue is resolved. 🙂

Loading

@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Nov 4, 2021

I am merging now since the PyPy is in the testing part and the install was successful (while it would have broken before).

Thanks, @jjerphan Let's track the setuptools issue.

Loading

@glemaitre glemaitre merged commit 953d101 into scikit-learn:main Nov 4, 2021
30 of 32 checks passed
Loading
@jtuyls jtuyls mentioned this pull request Nov 4, 2021
Closed
jjerphan added a commit to jjerphan/scikit-learn that referenced this issue Nov 5, 2021
@jjerphan
Revert "HOTFIX Don't use setuptools 58.5 which introduces regression (s…
bdd58d1 
…cikit-learn#21549)"

This reverts commit 953d101.

The fix for setuptools#21549 has been introduced in 58.5.3.

See: pypa/setuptools#2855 (comment)
@jjerphan jjerphan mentioned this pull request Nov 5, 2021
Merged
@jjerphan jjerphan deleted the setuptools_2849_workaround branch Nov 5, 2021
ogrisel pushed a commit that referenced this issue Nov 5, 2021
@jjerphan
Revert "HOTFIX Don't use setuptools 58.5 which introduces regression (#…
fac1ec8 
…21549)" (#21560)

This reverts commit 953d101.

The fix for setuptools#21549 has been introduced in 58.5.3.

See: pypa/setuptools#2855 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment