Opened 2 years ago

Closed 22 months ago

Last modified 22 months ago

#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) 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 2 years ago by gh-timokau

  • Status changed from new to needs_review

comment:2 Changed 2 years ago by git

  • Commit changed from c693f3967b72c3129a14de02eff465aa8ef3bd3d to e477287939cb5568e0e79930efa98c680df391e8

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e477287Make dochtml tests optional and enabled by default

comment:3 Changed 2 years ago by gh-timokau

  • Branch u/gh-timokau/htmldoc-optional deleted
  • Commit e477287939cb5568e0e79930efa98c680df391e8 deleted

comment:4 Changed 2 years ago by gh-timokau

  • Branch set to u/gh-timokau/htmldoc-optional
  • Commit set to e477287939cb5568e0e79930efa98c680df391e8

New commits:

a612c16Only add auto_optional_tags in addition to sage
e477287Make dochtml tests optional and enabled by default

comment:5 Changed 2 years ago by gh-timokau

  • Branch changed from u/gh-timokau/htmldoc-optional to u/gh-timokau/make-htmldoc-optional

comment:6 Changed 2 years ago by gh-timokau

Somehow trac has problems with my branch, I can't figure out why.

comment:7 Changed 2 years ago by gh-timokau

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 2 years ago by git

  • Commit changed from e477287939cb5568e0e79930efa98c680df391e8 to b08a0b691998f0462208ddbf3f16c22be6124dd0

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b08a0b6Make dochtml tests optional and enabled by default

comment:9 Changed 2 years ago by embray

We just pushed an update to Trac--there might still be some bugs after all. Probably not your branch's fault.

comment:10 Changed 2 years ago by gh-timokau

Ah, apparently it works now.

comment:11 Changed 2 years ago by jdemeyer

  1. You're adding bogus files like octave-workspace.
  1. I'm not too happy with the name htmldoc. It doesn't correspond to anything.
  1. As I mentioned on some other ticket, the # optional should only be on the first line of a test example.

comment:12 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:13 Changed 2 years ago by git

  • Commit changed from b08a0b691998f0462208ddbf3f16c22be6124dd0 to 51b549b628cbd1c0258db079646e26d7ff1e5cb4

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

51b549bMake dochtml tests optional and enabled by default

comment:14 Changed 2 years ago by gh-timokau

  • Status changed from needs_work to needs_review
  1. 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.
  1. I thought the makefile calls it htmldoc, but apparently its dochtml. Would that be better?
  1. 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:

51b549bMake dochtml tests optional and enabled by default

comment:15 Changed 2 years ago by git

  • Commit changed from 51b549b628cbd1c0258db079646e26d7ff1e5cb4 to 37a6f5e06c8ba18fc65c3e093dce2aa5fed7df62

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

37a6f5eMake dochtml tests optional and enabled by default

comment:16 Changed 2 years ago by git

  • Commit changed from 37a6f5e06c8ba18fc65c3e093dce2aa5fed7df62 to 7c41eed218db94fcea994576bde73d9ad9e78714

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7c41eedMake dochtml tests optional and enabled by default

comment:17 Changed 2 years ago by git

  • Commit changed from 7c41eed218db94fcea994576bde73d9ad9e78714 to 94ee038f3fdf02edb2929a1b40945b79cd412b73

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

94ee038Make dochtml tests optional and enabled by default

comment:18 Changed 2 years ago by git

  • Commit changed from 94ee038f3fdf02edb2929a1b40945b79cd412b73 to db7bf45015e736ca2383597a2c63e4fae97c9485

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

db7bf45Make dochtml tests optional and enabled by default

comment:19 follow-ups: Changed 2 years ago by jhpalmieri

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: Changed 2 years ago by 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 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 2 years ago by embray

It would also be great if ? could selectively pull docs from the internet whenever possible.

comment:22 in reply to: ↑ 20 ; follow-up: Changed 2 years ago by jhpalmieri

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 in abs?) 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 2 years ago by gh-timokau

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() and manual().

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.

Last edited 2 years ago by gh-timokau (previous) (diff)

comment:24 in reply to: ↑ 19 ; follow-up: Changed 2 years ago by 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.

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: Changed 2 years ago by 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.

comment:26 in reply to: ↑ 25 Changed 2 years ago by gh-timokau

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 2 years ago by gh-timokau

Are there any remaining objections?

comment:28 Changed 2 years ago by embray

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 2 years ago by gh-timokau

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 22 months ago by vdelecroix

  • Milestone changed from sage-8.3 to sage-8.4

update milestone 8.3 -> 8.4

comment:31 Changed 22 months ago by gh-timokau

  • Summary changed from Make htmldoc doctests optional to Make it possible to test sage without htmldoc

comment:32 Changed 22 months ago by embray

  • 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 22 months ago by vbraun

  • 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 22 months ago by jhpalmieri

  • Commit db7bf45015e736ca2383597a2c63e4fae97c9485 deleted

This is broken: tests marked optional - dochtml are not run by default. See #26110.

comment:35 in reply to: ↑ description Changed 22 months ago by jdemeyer

Replying to gh-timokau:

I also make sure that the optional py2 tag is only added if sage 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.

Note: See TracTickets for help on using tickets.