[MRG] Add documentation for supervised classes and add init docstrings to doc #115

wdevazelhes · 2018-08-20T12:37:02Z

Fixes #22

Here is a small proposal for adding some documentation for supervised classes.

wdevazelhes · 2018-08-20T14:31:59Z

Docstrings in __init__.py of classes currently does not appear in the doc, I wonder if we should put them in the class docstring (below the class name), or rather add some directives to sphinx not to ignore them

perimosocordiae · 2018-08-21T13:07:51Z

doc/index.rst

+Supervised Algorithms
+---------------------
+Supervised metric learning algorithms take as inputs points `X` and targets
+`y`, and learn to make points from the same class close to each other, and


target class labels

maybe "target labels" only, to include MLKR ?

Just realized the rest of the sentence ("[...]from the same class[...]") didn't include MLKR so I'll update this too

perimosocordiae · 2018-08-21T13:10:35Z

+1 for telling Sphinx to include the __init__ docstrings.

wdevazelhes · 2018-08-21T14:47:11Z

+1 for telling Sphinx to include the init docstrings.

According to this comment there are several ways to do that, I went for the one that includes the special-members because I found it easy to read and quite clean, but maybe option 1 (to include some code in conf.py), would be better, avoiding to add a line of code in every file ?

bellet

LGTM apart from a few nitpicks. Since the documentation will need to be adapted when we incorporate the new API, it is probably not worth spending too much time on improving the doc right now.

bellet · 2018-08-29T14:00:51Z

doc/index.rst

   metric_learn.nca
   metric_learn.lfda
+   metric_learn.mlkr
+
+Semi-Supervised Algorithms


I would rather use something like "Weakly supervised algorithms" to avoid confusion with the classic semi-supervised setting (i.e., a subset of points are labeled and other are not).

I agree
Done

bellet · 2018-08-29T14:08:52Z

doc/index.rst

+Supervised Algorithms
+---------------------
+Supervised metric learning algorithms take as inputs points `X` and target
+labels `y`, and learn to make points from the same class (for classification)


learn a distance metric that makes points...

bellet · 2018-08-29T14:13:59Z

doc/index.rst

+
+Semi-Supervised Algorithms
+--------------------------
+Semi-supervised algorithms do the same, but work on a weaker information about


maybe avoid "do the same" to have a more self consistent section
quick suggestion: "Weakly supervised algorithms work on weaker information about the data points than supervised algorithms. Rather than labeled points, they take as input similarity judgments on tuples of data points, for instance pairs of similar and dissimilar points. Refer to the documentation of each algorithm for its particular form of input data."

Yes I agree this is clearer
Done

bellet · 2018-08-29T14:16:01Z

metric_learn/itml.py

-    num_labeled : int, optional
-        number of labels to preserve for training
+    num_labeled : int, optional (default=np.inf)
+        number of labels to preserve for training. If np.inf (default),


what is meant by "preserve for training" needs to be clarified?

what about: "number of labeled points to keep for building pairs. Extra
labeled points will be considered unlabeled, and ignored as such.
Use np.inf (default) to use all labeled points." ?

In fact IMO this parameter is useless. I will open an issue

bellet · 2018-08-29T14:16:36Z

metric_learn/lsml.py

+
+    `LSML_Supervised` creates quadruplets from labeled samples by taking two
+    samples from the same class, and two samples from different classes, and
+    concatenating them. This way it builds quadruplets where the two first


", and concatenating them" seems useless

Agreed
Done

bellet · 2018-08-31T12:33:42Z

doc/index.rst


-Each metric supports the following methods:
+Note that each semi-supervised algorithm has a supervised version of the form


William de Vazelhes added 3 commits August 20, 2018 14:08

FEAT: Add documentation for supervised classes

2d4ef27

Merge branch 'master' into feat/add_doc_supervised_classes

1488794

Add doc for ITML, and put mlkr to the right place in index

f2558ed

perimosocordiae approved these changes Aug 21, 2018

View reviewed changes

William de Vazelhes added 5 commits August 21, 2018 15:45

Merge branch 'master' into feat/add_doc_supervised_classes

db08aea

FIX: update doc

95eb159

FIX: add __init__ docstrings in doc

16a55d7

FIX: Update doc to suit regression too

6fa5d83

STY: add points at end of docstrings

48d2007

wdevazelhes changed the title ~~[WIP] Add documentation for supervised classes~~ [MRG] Add documentation for supervised classes and add __init__ docstrings to doc Aug 21, 2018

bellet approved these changes Aug 29, 2018

View reviewed changes

MAINT: address scikit-learn-contrib#115 (review)

bd71af7

bellet reviewed Aug 31, 2018

View reviewed changes

DOC: fix semi to weakly supervised

b00cc20

bellet merged commit 22f60dd into scikit-learn-contrib:master Aug 31, 2018


		Each metric supports the following methods:
		Note that each semi-supervised algorithm has a supervised version of the form

[MRG] Add documentation for supervised classes and add __init__ docstrings to doc #115

[MRG] Add documentation for supervised classes and add __init__ docstrings to doc #115

Uh oh!

Conversation

wdevazelhes commented Aug 20, 2018

Uh oh!

wdevazelhes commented Aug 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wdevazelhes Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perimosocordiae commented Aug 21, 2018

Uh oh!

wdevazelhes commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bellet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[MRG] Add documentation for supervised classes and add init docstrings to doc #115

[MRG] Add documentation for supervised classes and add init docstrings to doc #115

wdevazelhes Aug 21, 2018 •

edited

Loading

wdevazelhes commented Aug 21, 2018 •

edited

Loading