[python-package] Guard docstring assignments for python -OO compatibility#12093
Closed
PavelGuzenfeld wants to merge 2 commits intodmlc:masterfrom
Closed
[python-package] Guard docstring assignments for python -OO compatibility#12093PavelGuzenfeld wants to merge 2 commits intodmlc:masterfrom
PavelGuzenfeld wants to merge 2 commits intodmlc:masterfrom
Conversation
Under python -OO (PYTHONOPTIMIZE=2), all docstrings are stripped and __doc__ attributes become None. This causes an AttributeError crash at import time when XGBClassifier.fit tries to call .replace() on None, and makes save_model/load_model display "None" as their docstrings via f-string interpolation. Guard each __doc__ assignment with a None check so that: - Normal mode: docstrings are preserved identically to before - python -OO mode: assignments are skipped, preventing the crash Closes dmlc#8537
Member
|
Why are there 2 PRs on this? |
Contributor
Author
|
The two PRs address the same issue (#8537) but take different approaches:
|
Contributor
Author
|
@RAMitchell Could you please reopen this PR? It was likely closed due to the confusion about the two PRs, but as clarified above, #12085 (the removed approach) is already closed and this one (#12093) is the corrected fix with a different approach. The underlying issue (#8537) is still present — XGBoost crashes on import under |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Running XGBoost with
python -OO(PYTHONOPTIMIZE=2) strips all docstrings,making
__doc__attributesNone. This causes anAttributeErrorcrash atimport time:
This PR wraps each
__doc__assignment with aNonecheck instead of removingthem, so documentation is fully preserved in normal mode and the crash is
avoided under
-OO.Why this approach (guard, not remove)
PR #12085 was correctly rejected —
it removed the assignments outright, which would have dropped method-level
docstrings for
save_model,load_model,XGBClassifier.fit, and Dask wrappersin normal Python runs.
This PR instead adds a one-line
if __doc__ is not None:guard before eachassignment. The result:
Addressing maintainer concern from #8537
There are only 5 assignments across 2 files. Each fix is a single
ifguard.No new tests needed — the fix is defensive by nature and cannot alter behavior in
normal mode (the guarded code is identical to the original).
Changes
python-package/xgboost/sklearn.py— guardsave_model.__doc__,load_model.__doc__, andXGBClassifier.fit.__doc__python-package/xgboost/dask/__init__.py— guardDaskXGBClassifier.predict_proba.__doc__andDaskXGBRanker.fit.__doc__Closes #8537