[MRG+1] MAINT Use magic to list documentation versions by jnothman · Pull Request #9841 · scikit-learn/scikit-learn

jnothman · 2017-09-27T15:02:46Z

Fixes #9838 with the exception of dealing with the NEWS section of index.rst. See #9838 (comment)

This makes the listing of documentation versions dynamic by:

using sphinx magic |version|
using sphinx javascript variable DOCUMENTATION_OPTIONS.VERSION
exploiting the current URL to determine if it's looking at the official current dev or stable dir
using CircleCI to construct a catalogue of all currently available documentation on scikit-learn.org, and to store it at http://scikit-learn.org/versions.html (this part could be nicer)
linking to versions.html instead of listing previous versions

jnothman · 2017-09-27T21:30:27Z

Circle and Travis errors are both scikit-learn build errors related to changing g++ versions?

g++: sklearn/svm/src/libsvm/libsvm_template.cpp
g++: error: unrecognized command line option ‘-fstack-protector-strong’
g++: error: unrecognized command line option ‘-fno-plt’
g++: error: unrecognized command line option ‘-fstack-protector-strong’
g++: error: unrecognized command line option ‘-fno-plt’

lesteve · 2017-09-27T21:37:25Z

My personal investigation starts in #9840 (comment). I am still hoping to work around the problem by using an older miniconda installer and not update conda, let's see ...

…oc-version

jnothman · 2017-09-28T00:20:18Z

At http://scikit-learn.org/circle?13937/documentation.html, see:

page heading
documentation menu
"other versions" listing

A nicer way to produce the "all versions" page (at least in terms of templating) would be to get sphinx to compile it as part of the dev documentation. This would mean automatically generating it by pulling scikit-learn.github.io and inspecting, which can be slow (although that would change with something like #7887 (comment)). It could alternatively be done with github contents API.

jnothman · 2017-09-28T04:41:53Z

http://scikit-learn.org/circle?13945/versions.html

lesteve · 2017-09-28T05:16:31Z

That seems nice, I'll try to take a look today.

jnothman · 2017-10-10T10:16:03Z

@lesteve, let me know if this seems reasonable by you or if we're better off just patching up the HTML ad-hoc.

lesteve · 2017-10-10T11:07:18Z

It's definitely on my todo list for the 0.19.1 release but I have not got around to look at it yet. Will try to do it soon.

jnothman · 2017-10-16T02:32:50Z

I'm taking this out of 0.19.1 it makes no significant benefit there.

rth · 2017-11-03T15:44:45Z

build_tools/circle/list_versions.py

+            if suffix == SUFFIXES[0]:
+                return "%d%s" % (quantity, suffix)
+            else:
+                return "%.1f%s" % (quantity, suffix)


Currently, the generated list of versions includes e.g. "(PDF 41.8MB)" I think it would be useful to add a space between the number and the suffix.

rth · 2017-11-03T15:52:36Z

build_tools/circle/list_versions.py

+        try:
+            html = urlopen(RAW_FMT % name).read().decode('utf8')
+        except Exception:
+            print('Failed to fetch %s' % (RAW_FMT % name), file=sys.stderr)


The latest CicleCI log has,

+ python build_tools/circle/list_versions.py Failed to fetch https://raw.githubusercontent.com/scikit-learn/scikit-learn.github.io/master/circle/documentation.html

that link is not correct.

What happens if fetching a resource fails? Does it mean that one of the linked version will be missing? Or that's not critical?

Perhaps we're better off with exceptions being critical and having a more curated glob.

rth · 2017-11-03T16:00:07Z

doc/documentation.rst

-                        <li><a href="http://scikit-learn.org/0.16/documentation.html">scikit-learn 0.16</a></li>
+                        <script>if (VERSION_SUBDIR != "stable") document.write('<li><a href="http://scikit-learn.org/stable/documentation.html">Stable version</a></li>')</script>
+                        <script>if (VERSION_SUBDIR != "dev") document.write('<li><a href="http://scikit-learn.org/dev/documentation.html">Development version</a></li>')</script>
+                        <li><a href="http://scikit-learn.org/dev/versions.html">Previous versions</a></li>


This doesn't work for circle ci preview. Can't one use relative paths here i.e. ./versions.html?

As below, it can't be a local link if it's to work on /stable and /0.19, etc. But the moment this is visible for master, the link will also work.

rth · 2017-11-03T16:06:13Z

doc/documentation.rst

  <div class="container-index">

-Documentation of scikit-learn 0.19.dev0
+Documentation of scikit-learn |version|


using sphinx magic |version|

Would you have a reference on how this works? Couldn't find it in sphinx docs...

Like many things in sphinx I stumbled upon it by accident, so now I can't remember how to get back there!

http://www.sphinx-doc.org/en/stable/config.html#confval-version hints at it, but I know it's elsewhere too

rth · 2017-11-03T16:10:50Z

build_tools/circle/list_versions.py

+
+
+digit_names = [k for k in dirs if k[:1].isdigit()]
+word_names = [k for k in dirs if not k[:1].isdigit()]


I'm not really able to follow what's happening in the 6 lines above. Maybe a few additional comments wouldn't hurt?

rth · 2017-11-03T16:20:24Z

doc/themes/scikit-learn/layout.html

-                <li><a href="http://scikit-learn.org/0.16/documentation.html">Scikit-learn 0.16</a></li>
-				<li><a href="{{ pathto('_downloads/scikit-learn-docs.pdf', 1) }}">PDF documentation</a></li>
+                <script>if (VERSION_SUBDIR != "stable") document.write('<li><a href="http://scikit-learn.org/stable/documentation.html">Stable version</a></li>')</script>
+                <script>if (VERSION_SUBDIR != "dev") document.write('<li><a href="http://scikit-learn.org/dev/documentation.html">Development version</a></li>')</script>


Couldn't this be done with sphinx templating ? My concern is that links dynamically changed with javascript are not so friendly toward search engines. Though it might not matter much in the case of scikit-learn...

Concern about spidering? Now it would have to spider through versions.html.... but that's not a great deal is it?

Sphinx templating... Lemme think.

I think for sphinx templating to work, we'd need to set a setting when building as to whether the document will be viewed as stable/dev/other. I don't think this is reasonable. When something moves from stable to historical, we don't want to recompile its docs. That's sort of the point of this PR.

I think for sphinx templating to work, we'd need to set a setting when building as to whether the document will be viewed as stable/dev/other. I don't think this is reasonable.

Aww yes, I see your point.

rth · 2017-11-03T16:22:41Z

doc/themes/scikit-learn/layout.html

-				<li><a href="{{ pathto('_downloads/scikit-learn-docs.pdf', 1) }}">PDF documentation</a></li>
+                <script>if (VERSION_SUBDIR != "stable") document.write('<li><a href="http://scikit-learn.org/stable/documentation.html">Stable version</a></li>')</script>
+                <script>if (VERSION_SUBDIR != "dev") document.write('<li><a href="http://scikit-learn.org/dev/documentation.html">Development version</a></li>')</script>
+                <li><a href="http://scikit-learn.org/dev/versions.html">Previous versions</a></li>


This also could be a local link?

No, it can't be a local link if it's to work on /stable and /0.19, etc.

jnothman · 2017-11-04T10:43:11Z

Thanks for the review, @rth! Do you think this is worthwhile overall, or too magic for new maintainers?

codecov · 2017-11-04T10:46:34Z

Codecov Report

Merging #9841 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #9841      +/-   ##
==========================================
+ Coverage   96.17%   96.17%   +<.01%     
==========================================
  Files         336      336              
  Lines       62406    62413       +7     
==========================================
+ Hits        60017    60024       +7     
  Misses       2389     2389

Impacted Files	Coverage Δ
sklearn/metrics/tests/test_ranking.py	`100% <0%> (ø)`	⬆️
sklearn/metrics/ranking.py	`98.31% <0%> (ø)`	⬆️
sklearn/metrics/tests/test_common.py	`99.52% <0%> (ø)`	⬆️

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 b9c79d4...edb44a1. Read the comment docs.

rth · 2017-11-10T13:38:00Z

doc/themes/scikit-learn/layout.html

+  <script>
+     VERSION_SUBDIR = (function(groups) {
+         return groups ? groups[1] : null;
+     })(location.href.match(/^https?:\/\/scikit-learn.org\/([^\/]+)/));


Checked that the above javascript regexp works as expected with the following code (run here),

<!DOCTYPE html> <html> <body> <button onclick="myFunction()">Match</button> <p id="demo"></p> <script> var url = "http://scikit-learn.org/0.19/modules/linear_model.html#lars-lasso" function matchUrl(groups) { return groups ? groups[1] : null; }; function myFunction() { var x = matchUrl(url.match(/^https?:\/\/scikit-learn.org\/([^\/]+)/)); document.getElementById("demo").innerHTML = x; } </script> </body> </html>

rth · 2017-11-10T13:38:46Z

@jnothman Sorry for the late reply.

Thanks for responding to all the comments.

Do you think this is worthwhile overall, or
too magic for new maintainers?

It would require some effort to understand how this works for new maintainers (and I am moderately enthusiastic about JS generated links) but I think it's really useful to have an always up to date list of the documentation for previous versions including the PDF.

Also, I don't have other ideas how the same results could be achieved in a simpler way. So generally this LGTM.

jnothman · 2017-11-11T11:19:19Z

thanks for the affirmation!

TomDLT

LGTM

TomDLT · 2017-11-17T15:16:20Z

doc/support.rst

-versions can be found here:
+This documentation is relative to |release|. Documentation for
+other versions can be found `here
+<http://scikit-learn.org/dev/documentation.html>`_.


I think http://scikit-learn.org/dev/versions.html would be more direct, no?

Yes, thanks!

jnothman · 2017-11-19T12:35:42Z

And feel free to merge if that seems to be consensus.

…oc-version

TomDLT · 2017-11-20T09:53:02Z

Thanks @jnothman !

qinhanmin2014

Will it be worth to also push this PR into 0.19 branch (stable doc)?

qinhanmin2014 · 2017-11-20T10:42:12Z

build_tools/circle/build_doc.sh

 # Build and install scikit-learn in dev mode
 python setup.py develop

+# TO BE UNCOMMENTED BEFORE MERGE


Seems strange for me. Ignore if that's intended.

Yep good catch @qinhanmin2014, I have pushed e2584c3

lesteve · 2017-11-20T11:05:29Z

Will it be worth to also push this PR into 0.19 branch (stable doc)?

Not sure, also I was wondering why the version list is only generated on master and not on 0.X.Y branches (if I am understanding the code correctly):

scikit-learn/build_tools/circle/build_doc.sh

Lines 119 to 123 in e2584c3

    
           if [[ "$CIRCLE_BRANCH" =~ ^master$ && -z "$CI_PULL_REQUEST" ]] 
        
           then 
        
               # List available documentation versions if on master 
        
               python build_tools/circle/list_versions.py > doc/versions.rst 
        
           fi

TomDLT · 2017-11-20T11:27:15Z

All generated docs link to the same page:
http://scikit-learn.org/dev/versions.html (#9841 (comment))

qinhanmin2014 · 2017-11-20T11:28:51Z

@lesteve Thanks :) I advocate to push it to 0.19 branch mainly bacause I think we should provide users a way to get access to the dev doc through the stable doc apart from typing dev in the url.

…9841)

jnothman · 2017-11-20T12:13:33Z

I've cherry-picked to 0.19.X (6554ed3)

lesteve · 2017-11-20T13:03:42Z

Small comment, on the stable doc, I think that "Previous versions" is a bit misleading because its lists 0.20.dev0:

Maybe "Other versions"?

qinhanmin2014 · 2017-11-20T13:06:51Z

Maybe "Other versions"?

But seems that it lists 0.19.1 itself. Maybe something like "All available versions" ?

lesteve · 2017-11-20T13:10:57Z

But seems that it lists 0.19.1 itself. Maybe something like "All available versions" ?

Good point, your suggestion seems fine. Better suggestions welcome!

jnothman · 2017-11-21T04:30:36Z

Shrug. Be bold. Make the change.

…9841)

* tag '0.19.2': Release 0.19.2 [MRG] Fix collections.abc deprecations with Python 3.7 (0.19.X backport) (scikit-learn#11509) FIX need to explicitly raise SkipTest MAINT disabled some tests failing under macOS and 32-bit Python on windows. MNT Release 0.19.2 with python 3.7 support (scikit-learn#11422) DOC previous versions -> all available versions (scikit-learn#10179) MAINT only build versions list on master branch [MRG+1] MAINT Use magic to list documentation versions (scikit-learn#9841) DOC Add Examples heading minor fixes to whatsnew fix release 0.19.1 whatsnew link (scikit-learn#9981)

* releases: Release 0.19.2 [MRG] Fix collections.abc deprecations with Python 3.7 (0.19.X backport) (scikit-learn#11509) FIX need to explicitly raise SkipTest MAINT disabled some tests failing under macOS and 32-bit Python on windows. MNT Release 0.19.2 with python 3.7 support (scikit-learn#11422) DOC previous versions -> all available versions (scikit-learn#10179) MAINT only build versions list on master branch [MRG+1] MAINT Use magic to list documentation versions (scikit-learn#9841) DOC Add Examples heading minor fixes to whatsnew fix release 0.19.1 whatsnew link (scikit-learn#9981)

* dfsg: Release 0.19.2 [MRG] Fix collections.abc deprecations with Python 3.7 (0.19.X backport) (scikit-learn#11509) FIX need to explicitly raise SkipTest MAINT disabled some tests failing under macOS and 32-bit Python on windows. MNT Release 0.19.2 with python 3.7 support (scikit-learn#11422) DOC previous versions -> all available versions (scikit-learn#10179) MAINT only build versions list on master branch [MRG+1] MAINT Use magic to list documentation versions (scikit-learn#9841) DOC Add Examples heading minor fixes to whatsnew fix release 0.19.1 whatsnew link (scikit-learn#9981)

jnothman added 3 commits September 28, 2017 00:28

Make Circle write a list of documentation versions

6c4a847

Merge branch 'list-versions' into doc-version

59df779

Use magic to list documentation versions

78ffada

jnothman added this to the 0.19.1 milestone Sep 27, 2017

jnothman added 3 commits September 28, 2017 01:05

Fix PDF docs link in documentation.rst

4314706

Fix bracketing of ternary operator

30254bd

Update maintainer docs

fee8abb

jnothman added 3 commits September 28, 2017 09:03

Comment

6e80b37

Leave VERSIONSUBDIR null instead of undefined

67d0a4b

Merge branch 'doc-version' of github.com:jnothman/scikit-learn into d…

e7447eb

…oc-version

jnothman added 4 commits September 28, 2017 12:35

Attempt to make previous versions listings dynamically generated ReST

06f1301

Add missing script

19e02d6

PEP8

2a4a413

I don't like ReST hyperlink syntax

da0e242

jnothman added 2 commits September 28, 2017 15:27

More idiomatic javascript

2625f24

Preface line for layout purpose

f562773

jnothman changed the title ~~[RFC/MRG] MAINT Use magic to list documentation versions~~ [MRG] MAINT Use magic to list documentation versions Oct 16, 2017

jnothman removed this from the 0.19.1 milestone Oct 16, 2017

jnothman mentioned this pull request Oct 24, 2017

DOC Add what's new for 0.19.1 #9983

Merged

jnothman mentioned this pull request Nov 2, 2017

Website: broken links to older versions #5911

Closed

Update support.rst

c9be779

rth reviewed Nov 3, 2017

View reviewed changes

Comments and minor tweaks

edb44a1

Remove unused import

ef32e8d

rth reviewed Nov 10, 2017

View reviewed changes

rth changed the title ~~[MRG] MAINT Use magic to list documentation versions~~ [MRG+1] MAINT Use magic to list documentation versions Nov 10, 2017

TomDLT approved these changes Nov 17, 2017

View reviewed changes

DOC improve existing link

fae5176

Merge branch 'doc-version' of github.com:jnothman/scikit-learn into d…

e372c96

…oc-version

TomDLT merged commit fd25481 into scikit-learn:master Nov 20, 2017

qinhanmin2014 reviewed Nov 20, 2017

View reviewed changes

jnothman added a commit to jnothman/scikit-learn that referenced this pull request Nov 20, 2017

[MRG+1] MAINT Use magic to list documentation versions (scikit-learn#…

6554ed3

…9841)

qinhanmin2014 mentioned this pull request Nov 21, 2017

DOC previous versions -> all available versions #10178

Merged

jwjohnson314 pushed a commit to jwjohnson314/scikit-learn that referenced this pull request Dec 18, 2017

[MRG+1] MAINT Use magic to list documentation versions (scikit-learn#…

87b710c

…9841)



		digit_names = [k for k in dirs if k[:1].isdigit()]
		word_names = [k for k in dirs if not k[:1].isdigit()]

Uh oh!

Conversation

jnothman commented Sep 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnothman commented Sep 27, 2017

Uh oh!

lesteve commented Sep 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnothman commented Sep 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnothman commented Sep 28, 2017

Uh oh!

lesteve commented Sep 28, 2017

Uh oh!

jnothman commented Oct 10, 2017

Uh oh!

lesteve commented Oct 10, 2017

Uh oh!

jnothman commented Oct 16, 2017

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!

jnothman Nov 4, 2017 • 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!

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!

jnothman commented Nov 4, 2017 via email

Uh oh!

codecov bot commented Nov 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rth commented Nov 10, 2017

Uh oh!

jnothman commented Nov 11, 2017 via email

Uh oh!

TomDLT left a comment

Choose a reason for hiding this comment

Uh oh!

TomDLT Nov 17, 2017 • 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!

jnothman commented Sep 27, 2017 •

edited

Loading

lesteve commented Sep 27, 2017 •

edited

Loading

jnothman commented Sep 28, 2017 •

edited

Loading

jnothman Nov 4, 2017 •

edited

Loading

codecov bot commented Nov 4, 2017 •

edited

Loading

TomDLT Nov 17, 2017 •

edited

Loading

TomDLT commented Nov 20, 2017 •

edited

Loading