#25382 closed defect (fixed)
py3: do not include the notebook documentation in sage
Reported by: | chapoton | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.5 |
Component: | notebook | Keywords: | |
Cc: | embray, jdemeyer, tscrim, fbissey, vbraun, kcrisman, jhpalmieri | Merged in: | |
Authors: | John Palmieri | Reviewers: | Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | 591e91a (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
because sagenb is not yet available for python3
Change History (73)
comment:1 Changed 4 years ago by
- Cc kcrisman added
comment:2 Changed 4 years ago by
- Branch set to public/25382
- Commit set to 501b4d20341a54646ab4c806942bd0ea74e1bc22
- Status changed from new to needs_info
comment:3 Changed 4 years ago by
Are we never going to get sagenb working in Python 3? I've just been assuming that for now we don't have it, but that we would eventually port it at least for now. Or are we truly abandoning sagenb for Python 3?
comment:4 Changed 4 years ago by
Well, if somebody wants to do the job of converting sagenb to full python3 compatibility.. I have no idea of the difficulty of the task.
comment:5 Changed 4 years ago by
I'm fine with removing sagenb when we drop Python2 support unless somebody wants to port it... The SageNB docs should just say that (and suggest to use the jupyter notebook)
comment:6 Changed 4 years ago by
If the ETA is still some time from now for Py2 drop that seems plausible. But hopefully it won't be that bad; I feel like #22431 was closer than we thought.
comment:7 Changed 4 years ago by
We should try in the ticket #22431 to see if we can re-activate sagenb in py3-sage. We decided some time ago to de-activate it as it was blocking the build of py3-sage. Try to-re-insert sagenb requires first #24269 to fix py3-sage. Then rewrite the spkg-install on top of #25394.
On the side-point to get rid of sagenb imports inside sage, tt would be helpful to positive-review #24994.
comment:8 Changed 4 years ago by
- Milestone changed from sage-8.3 to sage-duplicate/invalid/wontfix
- Status changed from needs_info to needs_review
let us close this one as invalid, as we are on the way towards building the doc on py3, including sagenb doc
comment:9 Changed 4 years ago by
I think this is still a good idea (for python2 and python3). Given that sagenb has been deprecated for a while, I think it would be a good idea to either just remove it or make it a optional spkg. #24994 is a nice step in that direction and this would be another. Why should the documentation of the deprecated sagenb be included in sage when the documentation of other spkgs isn't?
For context: sagenb is a pain to package. First because of https://github.com/sagemath/sagenb/issues/440 and now while updating flask I encountered more problems.
comment:10 Changed 4 years ago by
Deprecated in what sense? Unofficially, sure - but probably this would need to be a much bigger announcement and very obvious that it is removed, rather than the (very sensible) procedure of upgrade option we currently offer.
comment:11 Changed 4 years ago by
+1 to communicating the deprecation better, e.g. display a deprecation notice when you start sagenb. I don't really mind it being standard vs. optional, I just don't want somebody new to Sage get started with sagenb in 2018 just because it comes up by default.
comment:12 Changed 4 years ago by
Deprecated because I was told so in https://github.com/sagemath/sagenb/issues/440 (I was surprised there so I agree that it should be communicated better). Also the docs refer to it as the "legacy SageNB".
comment:13 follow-ups: ↓ 14 ↓ 15 Changed 4 years ago by
I don't think that anyone would be having it come up by default, the default is Jeroen's converter and pretty soon most people's default ends up the Jupyter. Maybe an intermediate thing to being an optional package (which is very bad for the kind of end users who would need sagenb!) would be to formally change the default interface to being Jupyter, with VERY clear instructions for how to continue conversion after that point sprinkled everywhere.
comment:14 in reply to: ↑ 13 Changed 4 years ago by
Replying to kcrisman:
Jeroen's converter
Just to give proper credit: Volker started that project and did most of the work. It's true that I also worked on it in order to make it the default interface, fixing various bugs and adding the "run SageNB" option.
comment:15 in reply to: ↑ 13 Changed 4 years ago by
Replying to kcrisman:
I don't think that anyone would be having it come up by default, the default is Jeroen's converter and pretty soon most people's default ends up the Jupyter.
I think that is even more of a reason to work towards making sagenb optional.
Maybe an intermediate thing to being an optional package (which is very bad for the kind of end users who would need sagenb!)
Are we sure there even are users that keep up with the latest version of sage (e.g. aren't using 6.0 forever or something) *and* still use sagenb? And even making sagenb optional wouldn't immediately make that impossible, it would just make it a tiny bit more inconvenient. That might be a good thing because then they might either complain here (showing us that there are such users) or realize that it might be time to look into the jupyter notebook.
would be to formally change the default interface to being Jupyter, with VERY clear instructions for how to continue conversion after that point sprinkled everywhere.
I agree.
comment:16 follow-up: ↓ 17 Changed 4 years ago by
<clear throat>Well in the math department at my university (university of Canterbury, New Zealand) they have reasonably recent installs of sage. But when I talk to them they also have hundreds of sagenb notebook because they have been using sage for quite some time. What they are after is literally a mass conversion tool. And even with one, the inertia of the old sagenb here sounds quite big.
comment:17 in reply to: ↑ 16 ; follow-up: ↓ 18 Changed 4 years ago by
Ah, good to know. What about that converter kcrisman was talking about?
comment:18 in reply to: ↑ 17 ; follow-up: ↓ 19 Changed 4 years ago by
Replying to gh-timokau:
Ah, good to know. What about that converter kcrisman was talking about?
There is probably a way to script it but it's just no one really want to take that job. I may have to do it for them at some point but that will be a tough sell.
comment:19 in reply to: ↑ 18 Changed 4 years ago by
Replying to fbissey:
Replying to gh-timokau:
Ah, good to know. What about that converter kcrisman was talking about?
There is probably a way to script it but it's just no one really want to take that job. I may have to do it for them at some point but that will be a tough sell.
Yeah I get that. We could still make the deprecation more obvious and further disentangle sagenb and sage. Maybe a big deprecation warning in 8.3 and then making it optional in 8.4 or something.
comment:20 Changed 4 years ago by
- Component changed from python3 to notebook
- Description modified (diff)
comment:21 Changed 4 years ago by
- Milestone changed from sage-duplicate/invalid/wontfix to sage-8.4
- Status changed from needs_review to needs_info
Removing sagenb doc would be one step towards building the doc of sage with python3. Otherwise, one needs to fix https://github.com/sagemath/sagenb/issues/454.
comment:22 Changed 4 years ago by
- Commit changed from 501b4d20341a54646ab4c806942bd0ea74e1bc22 to 820100e10f17de34633447e8c4a7d235c0185dc0
comment:24 Changed 4 years ago by
Is there a way to conditionally remove the docs, depending on whether Sage was built with Python 2 or 3?
comment:25 Changed 4 years ago by
Agreed.
comment:26 Changed 4 years ago by
By the way, with the branch here, make doc
and make doc-pdf
still try to build the notebook
directory in src/doc/en/reference
. So for this to work with Python 3, as long as SageNB is not Python 3 compatible, we have to rewrite /src/doc/en/reference/notebook/index.rst
as well. It may not matter what goes into that file when using Python 3, since it won't be referenced from anywhere.
comment:27 Changed 4 years ago by
My idea was something like this:
-
src/doc/en/reference/conf.py
diff --git a/src/doc/en/reference/conf.py b/src/doc/en/reference/conf.py index 9e55717545..9b5cf0559b 100644
a b import sys, os 15 15 from sage.env import SAGE_DOC_SRC, SAGE_DOC 16 16 sys.path.append(SAGE_DOC_SRC) 17 17 from common.conf import * 18 from sage_setup.docbuild.build_options import OMIT 18 19 19 20 ref_src = os.path.join(SAGE_DOC_SRC, 'en', 'reference') 20 21 ref_out = os.path.join(SAGE_DOC, 'html', 'en', 'reference') … … multidocs_is_master = True 61 62 # for 'static' and 'templates', and to deal with upgrades: 'sage', 62 63 # 'sagenb', 'media', and 'other'. 63 64 bad_directories = ['static', 'templates', 'sage', 'sagenb', 'media', 'other'] 65 for dir in OMIT: 66 if dir.startswith('reference' + os.sep): 67 bad_directories.append(dir.split(os.sep)[1]) 64 68 multidocs_subdoc_list = sorted([x for x in os.listdir(ref_src) 65 69 if os.path.isdir(os.path.join(ref_src, x)) 66 70 and x not in bad_directories]) -
src/doc/en/reference/index.rst
diff --git a/src/doc/en/reference/index.rst b/src/doc/en/reference/index.rst index 640df94c4a..91e6437045 100644
a b available below. 12 12 User Interface 13 13 ============== 14 14 15 * :doc:`Command Line Interface (REPL) <repl/index>` 15 .. only:: Python2 16 16 17 * For the jupyter notebook interface, see this `Documentation <https://jupyter-notebook.readthedocs.io/en/latest/notebook.html>`_. 17 * :doc:`Command Line Interface (REPL) <repl/index>` 18 * :doc:`Web Notebook <notebook/index>` 18 19 19 * For the legacy notebook interface, which is no longer actively maintained, see the `source repository <https://github.com/sagemath/sagenb>`_. 20 .. only:: Python3 21 22 * :doc:`Command Line Interface (REPL) <repl/index>` 23 * For the jupyter notebook interface, see this `Documentation <https://jupyter-notebook.readthedocs.io/en/latest/notebook.html>`_. 24 * For the legacy notebook interface, which is no longer actively maintained, see the `source repository <https://github.com/sagemath/sagenb>`_. 20 25 21 26 Graphics 22 27 ======== -
src/sage_setup/docbuild/__init__.py
diff --git a/src/sage_setup/docbuild/__init__.py b/src/sage_setup/docbuild/__init__.py index 5877b5b2a7..e750d4c9ab 100644
a b for a webpage listing all of the documents.''' % (output_dir, 669 669 670 670 for doc in os.listdir(refdir): 671 671 directory = os.path.join(refdir, doc) 672 if os.path.exists(os.path.join(directory, 'index.rst')): 672 if (os.path.exists(os.path.join(directory, 'index.rst')) 673 and os.path.join("reference", doc) not in OMIT): 673 674 n = len(os.listdir(directory)) 674 675 documents.append((-n, os.path.join(self.name, doc))) 675 676 -
src/sage_setup/docbuild/build_options.py
diff --git a/src/sage_setup/docbuild/build_options.py b/src/sage_setup/docbuild/build_options.py index 1168f02511..c35f71498f 100644
a b if PAPER: 16 16 else: 17 17 PAPEROPTS = "" 18 18 19 # The legacy Sage notebook is not compatible with Python 3, and its 20 # documentation will not build with Python 3, so omit it in that case. 21 # The command line options '-t Python3' and '-t Python2' define tags 22 # Python2 and Python3 which are used by Sphinx for conditional 23 # inclusion of text: text in a block marked '.. only:: Python2' will 24 # be included when Sphinx is invoked with '-t Python2'. See 25 # http://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#including-content-based-on-tags 26 # http://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-t 27 SAGE_PYTHON3 = os.environ.get("SAGE_PYTHON3", "no") == "yes" 28 if SAGE_PYTHON3: 29 TAGS = "-t Python3 " 30 OMIT.append(os.path.join("reference", "notebook")) 31 else: 32 TAGS = "-t Python2 " 33 19 34 #Note that this needs to have the doctrees dir 20 ALLSPHINXOPTS = SPHINXOPTS + " " + PAPEROPTS + " " 35 ALLSPHINXOPTS = SPHINXOPTS + " " + PAPEROPTS + " " + TAGS + " " 21 36 WEBSITESPHINXOPTS = "" 22 37 23 38 # Number of threads to use for parallel-building the documentation.
but it doesn't work with Python 3: even though we are supposed to be ignoring the directory src/doc/en/reference/notebook
, Sphinx still tries to use it in compiling indices, or something like that. Maybe there are more settings for ignoring files/directories that I could add.
comment:28 Changed 4 years ago by
So, you can define tags in reST which Sphinx will then pay attention to, for example
.. only:: my-tag stuff
Then if you run Sphinx with the option -t my-tag
, stuff
is included in the document. My first approach was to have tags Python2
and Python3
and run Sphinx with the corresponding -t Python[2|3]
. Unfortunately, the text in stuff
seems to be read even if the tag is not active, so any references to, for example, reference/notebook
eventually lead to problems with Python3.
The only way I can think to get around this is to have two different index.rst
files, one for use with each version of Python, and then create a symlink to the appropriate one when building the docs. One more point: any file ending in .rst
in a documentation directory seems to be read by Sphinx, even if it is not explicitly referenced anywhere. So the two different index.rst
files can't end in .rst
. So here is a branch which implements all of this. Works for me with Python 3 (if I get around the __div__
problem discussed at #25840).
comment:29 Changed 4 years ago by
- Commit changed from 820100e10f17de34633447e8c4a7d235c0185dc0 to 47ca8d69724c26b0f77e0eb0167304803b83c597
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
47ca8d6 | trac 25382: build the reference/notebook docs with Python 2 only: skip
|
comment:30 Changed 4 years ago by
By the way, even if this is the right approach, are symlinks the right thing to use? Are they available on Cygwin, for example? It's easy enough to copy the file instead.
comment:31 follow-up: ↓ 32 Changed 4 years ago by
Can't we just not include the sagenb documentation in both cases? Besides historical baggage, I don't see why sagenb should be treated differently from other dependencies here. It would simplify this change and would also simplify future deprecation.
comment:32 in reply to: ↑ 31 Changed 4 years ago by
Replying to gh-timokau:
Can't we just not include the sagenb documentation in both cases?
Hasn't been what the bulk of the discussion on this ticket has been about? It seems that not everyone agrees with this plan, so I suggested an alternative.
comment:33 Changed 4 years ago by
The way I read it most of this discussion is actually about weather or not we want to deprecate sagenb (#25837) and/or just disable it for python3. I don't see anybody arguing against the actual purpose of the ticket, just removing the documentation. Of course we would need to provide a user with a way to still access the documentation, just as we now do it with the jupyter notebook.
comment:34 Changed 4 years ago by
- Commit changed from 47ca8d69724c26b0f77e0eb0167304803b83c597 to 72c0cb2a8a0098e2f02f25c53ca6a914c402acab
Branch pushed to git repo; I updated commit sha1. New commits:
72c0cb2 | trac 25382: add versions of index.rst for Python2 and Python3.
|
comment:35 Changed 4 years ago by
I think that the decision of whether to remove the docs for SageNB in Python 2 belongs on sage-devel, not here. There is also the larger question of whether SageNB should be (or is?) officially deprecated. That also belongs on sage-devel. I don't think the few participants on this ticket should be making such policy decisions on our own.
comment:36 Changed 4 years ago by
I haven't been very active in this ticket. Nevertheless I'd like to voice an opinion now. Can anyone point me to another case where package A contains the documentation for package B. It would be so much easier if sageNB was shipping its own documentation. We wouldn't even have this conversation.
comment:37 Changed 4 years ago by
Of course the old Sage notebook used to be part of Sage, and (as you pointed out) it is still used as an interface to Sage. Anyway, can someone build its documentation and put is somewhere obvious on the sagenb github site? Then our reference manual could include a link to it (or skip it with Python 3, if we want conditional documentation depending on the Python version).
comment:38 Changed 4 years ago by
What @fbissey said was exactly the point I was also trying to make. sagenb is very much deprecated (as in no new features, minimal support, referred to as legacy
in the docs) but still supported for now. #25837 discusses the future status of that.
But independently of that, I don't see why sagenb has special status. We don't include the docs of other dependencies either. We could just explain the user where he can find the docs of sagenb, which are shipped separately (as part of sagenb).
comment:39 Changed 4 years ago by
- Dependencies set to #25852
We should only remove the docs if the notebook has effectively been deprecated.
comment:40 Changed 4 years ago by
Given that I am basically a glorified postdoc with funding (without anything in it earmarked for sagenb) running out in a year, I definitely do not want to spend time on sagenb. I am all for deprecating and removing it as soon as possible.
comment:41 Changed 4 years ago by
Moving the sagenb documentation to sagenb itself should be doable.
comment:42 follow-up: ↓ 45 Changed 4 years ago by
I would prefer the simpler solution to just remove the sagenb doc inconditionally..
And if someone feels able to find another place for this doc somewhere (readthedocs ?), please do !
comment:43 Changed 4 years ago by
- Commit changed from 72c0cb2a8a0098e2f02f25c53ca6a914c402acab to debe3ea26348de85d79536621cc68a080d3db404
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
debe3ea | trac 25382: remove the sagenb docs from the reference manual.
|
comment:44 Changed 4 years ago by
- Commit changed from debe3ea26348de85d79536621cc68a080d3db404 to 059de77749869b1a9a1e60d7ebb238316a0ac6cc
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
059de77 | trac 25382: remove the sagenb docs from the reference manual.
|
comment:45 in reply to: ↑ 42 Changed 4 years ago by
Replying to chapoton:
I would prefer the simpler solution to just remove the sagenb doc inconditionally..
Like this?
comment:46 Changed 4 years ago by
yes, just like that
comment:47 Changed 4 years ago by
I won't lie. I like it. That's all that's ever needed on sage side. If someone wants to do something sagenb side, then that's another tracker.
comment:48 Changed 4 years ago by
I agree.
comment:49 Changed 4 years ago by
I asked about the idea of removing the sagenb docs on sage-devel. Please add comments there.
comment:50 Changed 4 years ago by
- Dependencies #25852 deleted
no need for the dependency to #25852, I think
comment:51 Changed 4 years ago by
One week has passed on sage-devel, with only positive opinions. I think it's time to set this to positive. Agreed ?
comment:52 Changed 4 years ago by
Yes. That means the paperwork needs to be done. Authors and reviewers fields need to be filled in. And the ticket set to review and so on and so forth.
comment:53 Changed 4 years ago by
- Commit changed from 059de77749869b1a9a1e60d7ebb238316a0ac6cc to 591e91abedeee6a7a17882f04f78e7e544f2b9d5
Branch pushed to git repo; I updated commit sha1. New commits:
591e91a | trac 25382 remove remaining crosslinks
|
comment:55 follow-up: ↓ 57 Changed 4 years ago by
- Status changed from needs_info to needs_review
This is ready for review now. Patchbot is morally green.
comment:56 Changed 4 years ago by
OK, so we should keep a copy of the ready to read sagenb docs somewhere (linked at its github project). This is now https://github.com/sagemath/sagenb/issues/460
comment:57 in reply to: ↑ 55 Changed 4 years ago by
Replying to chapoton:
This is ready for review now. Patchbot is morally green.
Are these Python3 warnings/errors from https://patchbot.sagemath.org/log/25382/Ubuntu/16.04/x86_64/4.4.0-134-generic/atlas/2018-10-17%2009:49:44?plugin=python3 harmless? Just wondering - ik habla helemaal geen Espanol...
comment:58 Changed 4 years ago by
I think that the poor patchbot (not smart enough) is troubled by the removal of the files. Or maybe by the accented letters. But this should just be noise.
comment:59 follow-up: ↓ 60 Changed 4 years ago by
I've made a new release of sagenb, see #26499.
Should we also do downgrading sagenb to optional there?
comment:60 in reply to: ↑ 59 Changed 4 years ago by
comment:61 Changed 4 years ago by
On #26499 I've implemented installing sagenb (html) docs to $SAGELOCAL/share/docs/sagenb --- that's where they belong to by right.
comment:62 Changed 4 years ago by
I think that doc/en/reference/index.rst
should have a link to docs of "external" packages in SAGELOCAL/share/doc
(note that Sage's docs themselves are in
SAGELOCAL/share/doc/sage
).
Then in view of #26499 one can put the link to docs in
SAGELOCAL/share/doc/sagenb
comment:63 Changed 4 years ago by
Thanks very much everyone for handling all this properly so people can still find stuff.
comment:64 Changed 4 years ago by
- Milestone changed from sage-8.4 to sage-8.5
- Reviewers set to Dima Pasechnik
- Status changed from needs_review to positive_review
Looks good to me. I'll modify https://github.com/sagemath/sagenb/blob/master/README.rst to point to SAGELOCAL/share/doc/sagenb as the location for docs.
We should also find a way to put it up to doc.sagemath.org, but this is also not for this ticket.
comment:65 Changed 4 years ago by
FYI: graph_editor.py
imports sagenb.notebook.interact
, but has no pointer to the documentation of sagenb
.
We will certainly have to remove this tool when sagenb will be removed.
comment:66 Changed 4 years ago by
@dcoudert also see #25837.
comment:67 Changed 4 years ago by
- Branch changed from public/25382 to 591e91abedeee6a7a17882f04f78e7e544f2b9d5
- Resolution set to fixed
- Status changed from positive_review to closed
comment:68 Changed 3 years ago by
- Commit 591e91abedeee6a7a17882f04f78e7e544f2b9d5 deleted
Unfortunately now it seems that the interact documentation is not in Sage's usual online doc. That will break a number of links out there, plus interact is in many different interfaces to Sage, not just sagenb.
comment:69 Changed 3 years ago by
Well just like sagenb documentation should not be shipped with sage, sage documentation should not be in sagenb. That means some documentation has be migrated/rewritten in sage proper.
comment:70 Changed 3 years ago by
sagenb docs are installed in SAGE_LOCAL/share/doc/, among with many others.
This is however not at all reflected in the reference manual. I'm not sure how to achieve this with sphinx. By relative paths, like ../../
?
comment:71 Changed 3 years ago by
Not sure how its done right now, but intersphinx may be worth looking at: http://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html
comment:72 Changed 3 years ago by
I agree that intersphinx looks like the right tool for the job.
comment:73 Changed 3 years ago by
In any case, are we agreed that the interact documentation should be visible now?
I have no clear idea about what should be done here. Where will the sagenb doc live, if we get rid of it ?
New commits:
trying to remove sagenb doc