scikit-learn-contrib / metric-learn

Metric learning algorithms in Python
http://contrib.scikit-learn.org/metric-learn/
MIT License
1.4k stars 234 forks source link

Documentation: the import of KNeighborsClassifier is missing in "Getting Started" #258

Closed RobinVogel closed 5 years ago

RobinVogel commented 5 years ago

The getting started example at the bottom of this page misses the import of KNeighborsClassifier. Also, changing the presentation of code in the *.rst file from:

>>> print("hello")

to:

::
    print("hello")

as in supervised.rst allows to copy and paste the code as a block and try it straightaway.

I tried to generate the documentation to solve that, I wrote this line but I keep getting:

$ cd doc; make html
sphinx-build -b html -d _build/doctrees   . _build/html
Running Sphinx v2.2.0
loading translations [en]... done
loading pickled environment... done
[autosummary] generating autosummary for: auto_examples/index.rst, auto_examples/plot_metric_learning_examples.rst, auto_examples/plot_sandwich.rst, auto_examples/sg_execution_times.rst, getting_started.rst, index.rst, introduction.rst, metric_learn.rst, preprocessor.rst, supervised.rst, unsupervised.rst, user_guide.rst, weakly_supervised.rst
WARNING: [autosummary] failed to import 'metric_learn.Constraints': no module named metric_learn.Constraints

what is the process to build and test the website locally ?

wdevazelhes commented 5 years ago

Hi @RobinVogel , the copy-paste feature is nice ! Indeed, this line doesn't tell what is needed to make the make html work. The make html will show a lot of warnings, but indeed, there shouldn't be the warning failed to import 'metric_learn.Constraints'.. The process that seems to work for me is to install the following packages (in addition to numpy, scipy, and scikit-learn): sphinx, sphinx-gallery, numpydoc and sphinx_rtd_theme (maybe I'm missing some but a comprehensive error should tell which one to install). I also need to install metric-learn in editable mode: cd metric-learn; pip install -e .;. Maybe your pb comes from this last step because if I uninstall metric-learn I have the same warning message as you

RobinVogel commented 5 years ago

Thank you @wdevazelhes, I managed to generate the documentation using the "installation in editable mode" I did not know about. My documentation is very close from the one hosted by metric-learn, but differs slightly. I don't have experience with sphinx and if you happened to know where it comes from, it would help. I need to familiarize myself with sphinx in all cases.

My log and the generated documentation, that I inspect using a local server with python -m http.server. For example, my references in http://localhost:8000/supervised.html are much uglier than the official documentation.

I have many warnings of the type WARNING: 'any' reference target not found and I will start by seeing if I find some others that call for an installation and then study the others, to debug it by myself.

bellet commented 5 years ago

Here's what I usually do (as suggested by @wdevazelhes some time ago). I create a specific environment in anaconda and install the relevant packages:

conda create -n env_doc numpy scipy scikit-learn sphinx numpydoc pillow matplotlib
pip install sphinx-gallery sphinx_rtd_theme 

Then I install metric-learn in editable mode as suggested above:

cd metric-learn
pip install -e . 

Perhaps working in a clean environment solves the problem you're experiencing

bellet commented 5 years ago

@RobinVogel: regarding the missing import, I think it is likely that the doc contains other missing imports and also syntax errors. It would definitely be nice to fix everything by trying to run everything.

Note that to avoid more problems in the future we should have doctests, see #156

wdevazelhes commented 5 years ago

I have noticed that in fact I have the same problem in the references as you @RobinVogel, so I think before I was probably building the docs with an old sphinx version: using conda install sphinx=1 and rebuilding the docs indeed solves the problem of the references, but we should probably find what's going on to build with the latest version of sphinx, I'll raise an issue for that.

Regarding the any warnings, the thing is that we set in conf.py, default_role='any', to directly write stuff between brackets in the doc, without caring of whether it's a function or a class etc, and it will be found automatically in the package if it exists (ex: we can write just MMC instead of :class:metric_learn.mmc.MMC<MMC> (or sth like that)). But there's a lot of stuff we also write between brackets just for displaying them with the code syntax, like quadruplets for instance, which will raise warning since they cannot be ever found. Also, when we write transform for instance, there can be several functions depending on the class that implements it, so a warning is thrown too. So I guess we could disable the warnings but then if we mistype an actual reference we wouldn't see the warning. We could also revert to using the full syntax (like :class:metric_learn.mmc.MMC<MMC>), or maybe there's a clever way to deal with that...