#25345 closed enhancement (fixed)
Make it possible to test sage without htmldoc
Reported by: | gh-timokau | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.4 |
Component: | distribution | Keywords: | |
Cc: | Merged in: | ||
Authors: | Timo Kaufmann | Reviewers: | Erik Bray |
Report Upstream: | N/A | Work issues: | |
Branch: | db7bf45 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description
Because the html docs are non-essential and take up a lot of space, distributions usually make them an optional dependency of sage.
I am doing the same for nix and I would like to be able to test sage and the docs independent from each other.
This patch makes the doctests that depend on the htmldocs optional, but enables those doctests by default. So to exclude them, one has to explicitly specify --optional=sage
.
For that to work I also make sure that the optional py2
tag is only added if sage
is in the optionals list. I don't know if it was intended that way, but it was definitely surprising to me that all tests marked as py2
were *always* run, even with --optional=asdf-does-not-exists
.
(The same goes for --long
: I expected it to act as an AND
as in "in optionals list AND long", but instead it just includes all tests marked as long. I left that unchanged.)
Change History (35)
comment:1 Changed 4 years ago by
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
- Commit changed from c693f3967b72c3129a14de02eff465aa8ef3bd3d to e477287939cb5568e0e79930efa98c680df391e8
comment:3 Changed 4 years ago by
- Branch u/gh-timokau/htmldoc-optional deleted
- Commit e477287939cb5568e0e79930efa98c680df391e8 deleted
comment:4 Changed 4 years ago by
- Branch set to u/gh-timokau/htmldoc-optional
- Commit set to e477287939cb5568e0e79930efa98c680df391e8
comment:5 Changed 4 years ago by
- Branch changed from u/gh-timokau/htmldoc-optional to u/gh-timokau/make-htmldoc-optional
comment:6 Changed 4 years ago by
Somehow trac has problems with my branch, I can't figure out why.
comment:7 Changed 4 years ago by
Also looking back at the tests, it tells me Using --optional=gfortran,mpir,python2,sage
, so apparently adding htmldoc
didn't work. Does anybody know why?
comment:8 Changed 4 years ago by
- Commit changed from e477287939cb5568e0e79930efa98c680df391e8 to b08a0b691998f0462208ddbf3f16c22be6124dd0
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b08a0b6 | Make dochtml tests optional and enabled by default
|
comment:9 Changed 4 years ago by
We just pushed an update to Trac--there might still be some bugs after all. Probably not your branch's fault.
comment:10 Changed 4 years ago by
Ah, apparently it works now.
comment:11 Changed 4 years ago by
- You're adding bogus files like
octave-workspace
.
- I'm not too happy with the name
htmldoc
. It doesn't correspond to anything.
- As I mentioned on some other ticket, the
# optional
should only be on the first line of a test example.
comment:12 Changed 4 years ago by
- Status changed from needs_review to needs_work
comment:13 Changed 4 years ago by
- Commit changed from b08a0b691998f0462208ddbf3f16c22be6124dd0 to 51b549b628cbd1c0258db079646e26d7ff1e5cb4
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
51b549b | Make dochtml tests optional and enabled by default
|
comment:14 Changed 4 years ago by
- Status changed from needs_work to needs_review
- Sorry, I thought I double checked this time. Don't know how I missed this. By the way, should
octave-workspace
be in.gitignore
? Somehow that always gets generated when I run the tests.
- I thought the makefile calls it
htmldoc
, but apparently itsdochtml
. Would that be better?
- I thought that was only because it was an
if
or something. I didn't know a single#optional
will skip the whole test.
While changing (2) I noticed that there are some tests that are marked as long
and dochtml
. That means that its impossible to run them without running *all* other long
tests too. Is that behaviour of long
on purpose?
New commits:
51b549b | Make dochtml tests optional and enabled by default
|
comment:15 Changed 4 years ago by
- Commit changed from 51b549b628cbd1c0258db079646e26d7ff1e5cb4 to 37a6f5e06c8ba18fc65c3e093dce2aa5fed7df62
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
37a6f5e | Make dochtml tests optional and enabled by default
|
comment:16 Changed 4 years ago by
- Commit changed from 37a6f5e06c8ba18fc65c3e093dce2aa5fed7df62 to 7c41eed218db94fcea994576bde73d9ad9e78714
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
7c41eed | Make dochtml tests optional and enabled by default
|
comment:17 Changed 4 years ago by
- Commit changed from 7c41eed218db94fcea994576bde73d9ad9e78714 to 94ee038f3fdf02edb2929a1b40945b79cd412b73
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
94ee038 | Make dochtml tests optional and enabled by default
|
comment:18 Changed 4 years ago by
- Commit changed from 94ee038f3fdf02edb2929a1b40945b79cd412b73 to db7bf45015e736ca2383597a2c63e4fae97c9485
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
db7bf45 | Make dochtml tests optional and enabled by default
|
comment:19 follow-ups: ↓ 20 ↓ 24 Changed 4 years ago by
I'm not going to dwell on this, and if others think this is worth doing, I'm not going to argue about it, but I disagree with the premises of this ticket:
Because the html docs are non-essential
Documentation is an essential part of any software, so we should encourage everyone to build it or distribute it. Sage's philosophy has always been that documentation is a key part of the package, and an installation without it is incomplete. I think if someone chooses to not build the docs, then they should have the natural consequence of having some doctests fail.
and take up a lot of space, distributions usually make them an optional dependency of sage.
650MB is not that much space these days, is it?
comment:20 in reply to: ↑ 19 ; follow-up: ↓ 22 Changed 4 years ago by
Replying to jhpalmieri:
Documentation is an essential part of any software, so we should encourage everyone to build it or distribute it. Sage's philosophy has always been that documentation is a key part of the package, and an installation without it is incomplete. I think if someone chooses to not build the docs, then they should have the natural consequence of having some doctests fail.
Without html documentation sage still offers the ?
function (as in abs?
) that should suffice for a lot of use cases.
650MB is not that much space these days, is it?
Maybe not for you, but for somebody with a slow internet connection that wants to "quickly check out this software" its a lot of space/time.
comment:21 Changed 4 years ago by
It would also be great if ?
could selectively pull docs from the internet whenever possible.
comment:22 in reply to: ↑ 20 ; follow-up: ↓ 23 Changed 4 years ago by
Replying to gh-timokau:
Replying to jhpalmieri:
Documentation is an essential part of any software, so we should encourage everyone to build it or distribute it. Sage's philosophy has always been that documentation is a key part of the package, and an installation without it is incomplete. I think if someone chooses to not build the docs, then they should have the natural consequence of having some doctests fail.
Without html documentation sage still offers the
?
function (as inabs?
) that should suffice for a lot of use cases.
I think you should analyze this more carefully before making this change. "Should suffice": when will it or won't it? For "a lot of use cases": which ones won't it suffice for? Reading the tutorial is one which is especially relevant to beginners; are there others? When you first start Sage, it says to run help()
for help, and if you run this, it advertises two command which won't work without the docs: tutorial()
and manual()
.
Deciding not to include documentation is, in my experience, almost always a bad choice.
650MB is not that much space these days, is it?
Maybe not for you, but for somebody with a slow internet connection that wants to "quickly check out this software" its a lot of space/time.
It's not about me, it's compared to the size of the rest of the Sage installation.
comment:23 in reply to: ↑ 22 Changed 4 years ago by
I think you should analyze this more carefully before making this change.
This change does not disable documentation in any way. It is still enabled by default and tested by default. It simply makes it *possible* to disable.
"Should suffice": when will it or won't it? For "a lot of use cases": which ones won't it suffice for? Reading the tutorial is one which is especially relevant to beginners; are there others? When you first start Sage, it says to run
help()
for help, and if you run this, it advertises two command which won't work without the docs:tutorial()
andmanual()
.
Without any facts to back it up, I'd guess that most beginners start with the online documentation.
Deciding not to include documentation is, in my experience, almost always a bad choice.
Its the users choice.
650MB is not that much space these days, is it?
Maybe not for you, but for somebody with a slow internet connection that wants to "quickly check out this software" its a lot of space/time.
It's not about me, it's compared to the size of the rest of the Sage installation.
Well sagelib
(basically everything sage excluding dependencies and docs) only takes up 257M.
Of course realistically sage as a lot of dependencies that a user will not have pre-installed, but the documentation basically triples the minimum the user has to download.
comment:24 in reply to: ↑ 19 ; follow-up: ↓ 25 Changed 4 years ago by
Replying to jhpalmieri:
Documentation is an essential part of any software, so we should encourage everyone to build it or distribute it.
I don't think that this ticket changes that.
I see little harm in marking those tests optional
provided that make (p)test(long)
still test them by default.
comment:25 in reply to: ↑ 24 ; follow-up: ↓ 26 Changed 4 years ago by
Replying to jdemeyer:
Replying to jhpalmieri:
Documentation is an essential part of any software, so we should encourage everyone to build it or distribute it.
I don't think that this ticket changes that.
This ticket makes it easier for people to make bad choices. That's its whole point.
comment:26 in reply to: ↑ 25 Changed 4 years ago by
Replying to jhpalmieri:
Replying to jdemeyer:
Replying to jhpalmieri:
Documentation is an essential part of any software, so we should encourage everyone to build it or distribute it.
I don't think that this ticket changes that.
This ticket makes it easier for people to make bad choices. That's its whole point.
I think users should be able to make that choice. And distros already do allow them make that choice. The only thing that patch does is allowing them to stay closer to vanilla sage doing that (and pass the tests).
comment:27 Changed 4 years ago by
Are there any remaining objections?
comment:28 Changed 4 years ago by
I agree--shipping Sage without the full documentation is perfectly reasonable in my cases. We should just ensure that code which assumes that the documentation is installed has a reasonable fallback (e.g. tutorial()
returns a reasonable error message).
It should also be perfectly possible to run the tests without the docs (especially since the only tests that fail without them are tests that test the docs, so if the docs are not built it would be reasonable to skip those tests).
comment:29 Changed 4 years ago by
It should also be perfectly possible to run the tests without the docs (especially since the only tests that fail without them are tests that test the docs, so if the docs are not built it would be reasonable to skip those tests).
Yes that is exactly what this ticket accomplishes :) In fact I already applied this patch on the nixpkgs package and all doctests pass without documentation. I then run sage -t --optional=dochtml --all
after building the documentation. That also has the advantage of being able to test the base installation and build the documentation at the same time. That advantage is limited however, since in practice the doctests and the docbuild both use all the resources they can get anyways. It can be nice for a distributed build however.
comment:30 Changed 4 years ago by
- Milestone changed from sage-8.3 to sage-8.4
update milestone 8.3 -> 8.4
comment:31 Changed 4 years ago by
- Summary changed from Make htmldoc doctests optional to Make it possible to test sage without htmldoc
comment:32 Changed 4 years ago by
- Reviewers set to Erik Bray
- Status changed from needs_review to positive_review
We should ensure that built-ins like tutorial()
behave reasonably when the docs are not installed, but the fact remains that Sage is already shipped by default in many cases without the docs, so that's a separate issue. In the meantime we should certainly allow the tests to pass without docs.
I would kind of like it if dochtml
were automatically disabled if the docs are not found, but that could also hide a legitimate problem. So I'm not sure what to do there.
comment:33 Changed 4 years ago by
- Branch changed from u/gh-timokau/make-htmldoc-optional to db7bf45015e736ca2383597a2c63e4fae97c9485
- Resolution set to fixed
- Status changed from positive_review to closed
comment:34 Changed 4 years ago by
- Commit db7bf45015e736ca2383597a2c63e4fae97c9485 deleted
This is broken: tests marked optional - dochtml
are not run by default. See #26110.
comment:35 in reply to: ↑ description Changed 4 years ago by
Replying to gh-timokau:
I also make sure that the optional
py2
tag is only added ifsage
is in the optionals list.
This change doesn't really make sense to me. I think it's a feature that # py2
tests are always run on Python 2, regardless of the --optional
option. So I'm reverting this part in #26110.
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
Make dochtml tests optional and enabled by default