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

[breaking] Remove duplicated predict functions, Fix attributes IO. #6593

Merged
merged 1 commit into from Jan 13, 2021

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Jan 11, 2021

  • Use inheritance to remove duplicated code.
  • Fix attributes not being restored.
  • Rename all data to X. [breaking]

@hcho3
Copy link
Collaborator

hcho3 commented Jan 12, 2021

This PR is only breaking for the users who specified the data argument as a keyword argument?

@trivialfis
Copy link
Member Author

This PR is only breaking for the users who specified the data argument as a keyword argument?

Yes.

@trivialfis
Copy link
Member Author

trivialfis commented Jan 12, 2021

This is part of cleaning up the Python code base. #6594 is also used for cleaning up, but happens to fix an old hidden bug. One last big refactor is in #6591 . The goal is to bring the dask, sklearn and core modules together.

@trivialfis
Copy link
Member Author

trivialfis commented Jan 12, 2021

@hcho3 This turned up to be another fix for attributes like best_ntree_limit not being restored properly after reading model.

Update: Didn't recall that these things are not supposed to be saved. Worked on saving booster feature names and types. Need some time to get it done as removing feature name generation breaks quite a few things.

* Fix attributes not being restored.
* Rename all `data` to `X`. [breaking]
@trivialfis trivialfis changed the title [breaking] Remove duplicated predict functions. [breaking] Remove duplicated predict functions, Fix attributes IO. Jan 12, 2021
@codecov-io
Copy link

codecov-io commented Jan 12, 2021

Codecov Report

Merging #6593 (a2635af) into master (03cd087) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6593      +/-   ##
==========================================
- Coverage   80.44%   80.33%   -0.12%     
==========================================
  Files          13       13              
  Lines        3657     3656       -1     
==========================================
- Hits         2942     2937       -5     
- Misses        715      719       +4     
Impacted Files Coverage Δ
python-package/xgboost/core.py 81.57% <100.00%> (+0.13%) ⬆️
python-package/xgboost/sklearn.py 89.17% <100.00%> (-0.42%) ⬇️
python-package/xgboost/training.py 95.37% <100.00%> (+0.04%) ⬆️
python-package/xgboost/tracker.py 93.98% <0.00%> (-1.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03cd087...a2635af. Read the comment docs.

python-package/xgboost/sklearn.py Show resolved Hide resolved
@trivialfis trivialfis merged commit 0027220 into dmlc:master Jan 13, 2021
@trivialfis trivialfis deleted the remove-predict branch January 13, 2021 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants