New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup console output during build #250

Merged
merged 7 commits into from Jun 27, 2017

Conversation

Projects
None yet
3 participants
@QuLogic
Contributor

QuLogic commented May 30, 2017

This PR tries to reduce the console output during a build, as well as pass a lot of status message through the Sphinx logging API. This gives the immediate benefit of matching styling with Sphinx easily. I tried to add colours to various items to match Sphinx appearance, but I'm open to suggestions as to the logging level of the messages.

Before:
asciicast
After:
asciicast

Since sphinx-gallery's documentation prints a lot, it might not totally clear, so here's a Matplotlib doc build with the patched branch:
asciicast

Fixes #230.

@lesteve

This comment has been minimized.

Contributor

lesteve commented Jun 1, 2017

This looks quite good. Have you tested this with an old sphinx version e.g. 1.5 or even 1.4?

@QuLogic

This comment has been minimized.

Contributor

QuLogic commented Jun 2, 2017

I tried with 1.4.8, but I'm not sure how old a sphinx is supported.

@Titan-C

This comment has been minimized.

Member

Titan-C commented Jun 10, 2017

I tried with 1.4.8, but I'm not sure how old a sphinx is supported.

Sphinx-1.4.8 is on ubuntu16.10. On travis our oldest supported platform is the ubuntu14.04 and there we enforce sphinx-1.4.9(not 1.2.2 as in the distribution) and that is because the huge problems Sphinx-1.5 caused(and story repeats now with Sphinx-1.6), so we test on a version just before that. Up until that point things didn't change so drastically in Sphinx. My point of view would be not to try to support even older version, and only add a new entry into travis a test for Sphinx-1.5 on a ubuntu14 machine which is the oldest we support.

@Titan-C

This comment has been minimized.

Member

Titan-C commented Jun 10, 2017

I will reference to #65, where conclusion was to have an indicator of progress through stdout, and that error messages should be easy to find.
This PR is great at reducing the verbose output we have, I really like it, but looking at the travis output and the video of the output I'm concerned that you loose the information of which example is producing the text output during the build. If someone ever reads the build log that would make it very confusing. On the other hand since #97 we repeat the traceback of the failing examples which is really important and worth reading.

I would really like to keep information of which example gave the output, instead of it being text polluting the logs, without any context
Second, unsure how easy it would be to implement, I'm in favour our reducing output so I would also like to send all this output to the Sphinx-verbose logger(or log) and require the user to be explicit if he wants to see all this output during build.

@Titan-C

This is nice. I would also rename compat.py to sphinx_compatibility.py
I prefer more explicit naming

@pytest.fixture
def fakeapp():

This comment has been minimized.

@Titan-C

Titan-C Jun 10, 2017

Member

can you rename it to fakesphinxapp.

@pytest.fixture
def logcap():

This comment has been minimized.

@Titan-C

Titan-C Jun 10, 2017

Member

rename to:

  • capture_logs
  • log_collector
  • log_grabber
  • any better name that makes it explicit what this does or is intended for which use.
@QuLogic

This comment has been minimized.

Contributor

QuLogic commented Jun 10, 2017

I'm not sure it would be possible to keep outputs together without capturing output from the examples. And I'm thinking that perhaps if that were done, the output should go in the HTML (also?).

@QuLogic QuLogic force-pushed the QuLogic:quiet branch from b54025a to c0fd13d Jun 10, 2017

@QuLogic QuLogic force-pushed the QuLogic:quiet branch from c0fd13d to ded6b0d Jun 10, 2017

@QuLogic

This comment has been minimized.

Contributor

QuLogic commented Jun 10, 2017

Oh, I didn't realize that there's already stdout capture and passed through a tee to the console. I just took out the tee and logged it as verbose; then it can be seen by setting Sphinx logging to verbose.

@Titan-C Titan-C merged commit 8441890 into sphinx-gallery:master Jun 27, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Titan-C

This comment has been minimized.

Member

Titan-C commented Jun 27, 2017

Thank you for working on this. Merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment