Skip to content
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

Doc test no pydata #20720

Closed
wants to merge 7 commits into from
Closed

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Jul 22, 2021

PR Summary

DO NOT MERGE..

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making sure it won't be merged ;)

@jklymak
Copy link
Member Author

jklymak commented Jul 22, 2021

  • OK, so took 26 minutes with the pydata theme commented out.
  • Other recent builds took 40 minutes

I'd say the pydata theme is definitely slowing things down

@jklymak
Copy link
Member Author

jklymak commented Jul 22, 2021

The point raised on gitter about optipng is a good one. Trying with no compressing of thumbnails.

@jklymak
Copy link
Member Author

jklymak commented Jul 22, 2021

  • turning off compression dropped the build time to 18 minutes (pydata theme still off)

@jklymak
Copy link
Member Author

jklymak commented Jul 23, 2021

(BTW, thanks @larsoner for the skip commands!)

@jklymak
Copy link
Member Author

jklymak commented Jul 23, 2021

Pinning pydata_theme fails, and took 48 minutes to do so. Of course its possible the failure is causing the slowness, but I would guess not.

@jklymak
Copy link
Member Author

jklymak commented Jul 23, 2021

18c4fde (collapse navigation = True and no compression) took 26 minutes. So collapsing the navigation helps substantially.

@larsoner
Copy link
Contributor

From https://pydata-sphinx-theme.readthedocs.io/en/latest/user_guide/configuring.html#configure-the-navigation-depth-and-collapsing-of-the-sidebar it looks like the default depth is 4, maybe restricting to 2 would be faster enough?

html_theme_options = {
  "navigation_depth": 2
}

@larsoner
Copy link
Contributor

(or maybe this won't speed things up at all, not sure...)

@jklymak
Copy link
Member Author

jklymak commented Jul 23, 2021

Ha, well that one took over an hour. I'm nit sure I trust circle to do these tests, so I guess I'll do them on a local machine....

@jklymak
Copy link
Member Author

jklymak commented Jul 23, 2021

OK, here is the results of tests for my machine:

all sphinx 3.5.3
sphinx-gallery                 0.9.0.dev0 
serial build (no parallel)

make clean each time
make html

compress_images: ('thumbnails', 'images'),
collapse_navigation: False
pydata-sphinx-theme:            0.6.3
commit: master
result: 2010.13s  = 33.5 minutes

compress_images: ('thumbnails', 'images'),
collapse_navigation: True
pydata-sphinx-theme:            0.6.3
commit: master
result: 1639.42s  = 27.3 minutes

compress_images: Off!
collapse_navigation: True
pydata-sphinx-theme:            0.6.3
commit: master
result: 1322.72s  = 22 minutes

compress_images: True
commit: 7d77bb9670  (no theme)
result: 1232.57s = 20.54 minutes

compress_images: False
commit: 7d77bb9670  (no theme)
result: 911.00s = 15.2 minutes

So in sum the breakdown is basically:

  • fastest = 15 minutes
  • new theme = +7 minutes
  • compress images = +5 minutes
  • uncollapsed navigation = +5 minutes

Is there any reason to compress the images on CI?

We can run CI with the navigation collapsed. Those combined should get us from 33 minutes to 22 or 23 minutes. Thats still 50% slower than without the new theme, but...

@timhoffm
Copy link
Member

Thanks @jklymak for doing all the tests!

Is there any reason to compress the images on CI?

I don't think there is a strong reason. It makes the devdocs upload larger and viewing the devdocs costs more bandwidth, but that's bearable if the built is significantly faster.

@jklymak jklymak mentioned this pull request Jul 23, 2021
7 tasks
@jklymak
Copy link
Member Author

jklymak commented Jul 23, 2021

.. sure well lets see; if it adds 5 minutes to the upload time, its not worth doing ;-)

@jklymak
Copy link
Member Author

jklymak commented Jul 24, 2021

Closed by #20727

@jklymak jklymak closed this Jul 24, 2021
@jklymak jklymak deleted the doc-test-no-pydata branch July 24, 2021 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants