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

Revert "XGBoost: Temporary workarounds #5649

Merged
merged 3 commits into from
Oct 25, 2021

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Oct 17, 2021

It reverses workaround from pull requests: #5653 and #5613

@codecov
Copy link

codecov bot commented Oct 17, 2021

Codecov Report

Merging #5649 (ef0c6a5) into master (f4b412d) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #5649   +/-   ##
=======================================
  Coverage   86.05%   86.05%           
=======================================
  Files         315      315           
  Lines       65958    65954    -4     
=======================================
  Hits        56759    56759           
+ Misses       9199     9195    -4     

@janezd
Copy link
Contributor Author

janezd commented Oct 17, 2021

Ha! XGBoost didn't fix it, apparently. :(

@janezd janezd changed the title Revert "XGBoost: Temporary workaround before release of v1.5" [RFC] Revert "XGBoost: Temporary workaround before release of v1.5" Oct 17, 2021
@janezd janezd added the needs discussion Core developers need to discuss the issue label Oct 17, 2021
@PrimozGodec
Copy link
Contributor

I suggest that we keep the workaround utill the next xgboost release #5653 and report the issue to xgboost. I haven't had time to find a minimal example yet.

@PrimozGodec
Copy link
Contributor

It seems that the error is caused by in combination with scikit-learn=1.0.0. dmlc/xgboost#7355
Thanks, @markotoplak for help with debugging.

@janezd janezd closed this Oct 22, 2021
@markotoplak
Copy link
Member

markotoplak commented Oct 25, 2021

@PrimozGodec, scikit-learn just released 1.0.1. Does it fix the issue? Judging by
scikit-learn/scikit-learn#21227, it should could.

@PrimozGodec
Copy link
Contributor

Oh really. Yes, it should fix the issue. Today in the morning I was wondering when it will happen. 📦

@PrimozGodec PrimozGodec reopened this Oct 25, 2021
@PrimozGodec
Copy link
Contributor

PrimozGodec commented Oct 25, 2021

Let's try if tests pass.

@PrimozGodec PrimozGodec changed the title [RFC] Revert "XGBoost: Temporary workaround before release of v1.5" [RFC] Revert "XGBoost: Temporary workarounds Oct 25, 2021
@PrimozGodec PrimozGodec changed the title [RFC] Revert "XGBoost: Temporary workarounds Revert "XGBoost: Temporary workarounds Oct 25, 2021
requirements-core.txt Outdated Show resolved Hide resolved
@PrimozGodec
Copy link
Contributor

It is ready for review.

Co-authored-by: Primož Godec <p.godec9@gmail.com>
@markotoplak markotoplak merged commit 9d5ea64 into biolab:master Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Core developers need to discuss the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants