[python][scikit-learn] change MRO

[StrikerRUS]

Refer to

Note that Mixins like RegressorMixin must come before base classes in the MRO for _get_tags() to work properly.
https://scikit-learn.org/stable/whats_new/v0.23.html#sklearn-utils

[StrikerRUS]

@henry0312 Do you have any ideas why changing base classes order causes Sphinx to fail? To be honest, I'm out of ideas...

[guolinke]

@StrikerRUS this is blocked by the unexpected error?

[StrikerRUS]

@guolinke Surprisingly, changing the order of parent classes causes that Sphinx cannot build the documentation for the whole Python API. I spent two days digging this problem but unfortunately didn't find any ideas why it happens.

[StrikerRUS]

I remember @hayesall was very experienced in Sphinx and helped us a lot with Python docs config some time ago. May we ask you @hayesall to take a look at the current issue? Thanks in advance!

[jameslamb]

I spent a little time trying this this morning too. Couldn't get to a root cause, but I can add the specific error message here since I think that might help.

After the changes in this PR, running cd docs && make html results in the following error

Running Sphinx v3.3.1
loading pickled environment... done
[autosummary] generating autosummary for: Python-API.rst

Warning, treated as error:
[autosummary] failed to import 'lightgbm.Booster': no module named lightgbm.Booster
make: *** [html] Error 2

But python -c "from lightgbm import Booster" works without issue, so I don't think I just have a broken install.

[StrikerRUS]

@jameslamb

Yes, the exact error can be seen in CI logs:
https://travis-ci.org/github/microsoft/LightGBM/jobs/752595319#L553

[austin3dickey]

Hey @jameslamb, I saw your tweet. This error message is a confusing one but something like that happens whenever sphinx-autodoc can't import the package for some reason. Simplifying a lot, it tries stuff like this:

from sphinx.ext.autodoc.importer import import_module

try:
    thing = getattr(import_module("lightgbm"), "Booster")
except Exception:
    pass

try:
    thing = import_module("lightgbm.Booster")
except Exception:
    raise <unhelpful_error>

and when that fails it raises your error, suppressing the traceback of the actual reason why it can't import it.

Perhaps you could try inserting a line in your CI script right before make html that looks like

python -c 'from sphinx.ext.autodoc.importer import import_module; print(getattr(import_module("lightgbm"), "Booster"))'

and see if that prints a more helpful error message?

[jameslamb]

@austin3dickey thanks for the suggestion! That helped, and in a way that might surprise you.

Running this didn't fail

python -c 'from sphinx.ext.autodoc.importer import import_module; print(getattr(import_module("lightgbm"), "Dataset"))'

But what it printed gave me a hint

<class 'lightgbm.basic.Booster'>

This told me that maybe the problem was that somehow sphinx couldn't see lightgbm.basic.Booster even though it was available as lightgbm.Booster based on the way we have __init__.py.

Long story short, I tried a bunch of things and eventually was able to get past these errors by changing all the entries in https://github.com/microsoft/LightGBM/blob/master/docs/Python-API.rst to this:

.. autosummary::
    :toctree: pythonapi/

    .. autosummary:: Dataset
    .. autosummary:: Booster
    .. autosummary:: CVBooster

This then showed me what I think the "real" error is:

Warning, treated as error:
autodoc: failed to import class 'basic.Booster' from module 'lightgbm'; the following exception was raised:
Traceback (most recent call last):
File "/home/jlamb/miniconda3/envs/lgb/lib/python3.6/site-packages/sphinx/ext/autodoc/importer.py", line 66, in import_module
return importlib.import_module(modname)
File "/home/jlamb/miniconda3/envs/lgb/lib/python3.6/importlib/init.py", line 126, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "", line 994, in _gcd_import
File "", line 971, in _find_and_load
File "", line 955, in _find_and_load_unlocked
File "", line 665, in _load_unlocked
File "", line 678, in exec_module
File "", line 219, in _call_with_frames_removed
File "/home/jlamb/repos/open-source/LightGBM/docs/../python-package/lightgbm/init.py", line 14, in
from .sklearn import LGBMModel, LGBMRegressor, LGBMClassifier, LGBMRanker
File "/home/jlamb/repos/open-source/LightGBM/docs/../python-package/lightgbm/sklearn.py", line 760, in
class LGBMRegressor(_LGBMRegressorBase, LGBMModel):
TypeError: Cannot create a consistent method resolution
order (MRO) for bases object, LGBMModel

I'm not sure what to do yet, but that at least is a more informative error than the one we were getting from sphinx originally!

[shiyu1994]

@StrikerRUS Has the problem been solved. Is possible that this is due to the import error of sklearn? Because I've observe the following in my local VSCode environment.
First, exchanging the order of _LGBMRegressorBase and LGBMModel results in this

And here's the definition of _LGBMRegressorBase in compat.py

But when I manually import RegressorMixin, and replace _LGBMRegressorBase with RegressorMixin in the inheritance of LGBMRegressor, the error disappears.

On the contrary, if I replace _LGBMRegressorBase with object, the error persists.

So it is possible that in compat.py, there's an import error when importing from sklearn, thus _LGBMRegressorBase becomes object, which causes the error?

Above is just a guess from simple trial-and-error.

[StrikerRUS]

@shiyu1994 Brilliant investigation! This helps a lot! I guess that mock is somehow involved into this process of incorrect imports.

LightGBM/docs/conf.py

Lines 40 to 44 in d517ba1

	# -- mock out modules
	MOCK_MODULES = ['numpy', 'scipy', 'scipy.sparse',
	'sklearn', 'matplotlib', 'pandas', 'graphviz', 'dask', 'dask.distributed']
	for mod_name in MOCK_MODULES:
	sys.modules[mod_name] = Mock()

[StrikerRUS]

Finally got a chance to work on this. The thing was in that in case of scikit-learn unavailability LGBMClassifier or LGBMRegressor was inheriting from (object, object) actually. Here is a more detailed explanation of the problem: https://stackoverflow.com/questions/29214888/typeerror-cannot-create-a-consistent-method-resolution-order-mro.

Thanks a lot to all participants! @shiyu1994 Your investigation was great!

[jameslamb]

🚀 great work! Such a simple-looking diff, but I know this was a really complicated problem.

I fetched this branch and built the docs locally. I clicked around the "Python API" docs and everything looks correct. Given that and the fact that CI jobs are passing, I think this PR can be merged.

[jameslamb]

Thanks again for this, @StrikerRUS ! I'm very glad to see this merged 😀

[github-actions]

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.