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

ENH: Write all computation times #1197

Merged
merged 8 commits into from Sep 21, 2023
Merged

Conversation

larsoner
Copy link
Contributor

For a while I've wanted a full listing of example computation times. This PR writes them to the src_dir root as sg_execution_times.html, which I think is hopefully safe enough.

Snuck in some cleanups of pytest.mark.parametrize so our test names can be a bit shorter in the terminal, too.

Copy link
Contributor

@lucyleeow lucyleeow left a comment

Choose a reason for hiding this comment

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

Thanks @larsoner ! Doc nitpicks.

I think we could add a note to the 'getting started' around this section:

After building your documentation, ``gallery_dirs`` will contain the following

about the root sg_execution_times.rst file?

sphinx_gallery/gen_gallery.py Outdated Show resolved Hide resolved
sphinx_gallery/gen_gallery.py Outdated Show resolved Hide resolved
sphinx_gallery/gen_rst.py Outdated Show resolved Hide resolved
sphinx_gallery/gen_gallery.py Outdated Show resolved Hide resolved
sphinx_gallery/gen_gallery.py Outdated Show resolved Hide resolved
sphinx_gallery/gen_gallery.py Outdated Show resolved Hide resolved
sphinx_gallery/gen_gallery.py Outdated Show resolved Hide resolved
@@ -861,9 +867,9 @@ def _rerun(
# - auto_examples/index
# - auto_examples/plot_numpy_matplotlib
if how == "modify":
n_ch = "[3-7]"
n_ch = "[3-9]"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we amend the comment above to add the global execution file to "Ones that can change on stale:". Also I'm lost, why is this not +1 (8) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not totally sure why I needed the extra one here but I also think it's okay to be lenient

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... actually updating the comment made it clear I think, pushing

@larsoner
Copy link
Contributor Author

I went to fix the unrelated graphviz error and found we're still only requiring 3.7 so I bumped that to 3.8 in a couple of places. I also saw some cruft in the doc/Makefile about theme so I got rid of that, and in doing so renamed rtd_html to html in CircleCI since we don't use RTD or change our theme anymore. I'll keep iterating until CIs are happy but I think I incorporated the requested changes so you can look @lucyleeow !

@larsoner
Copy link
Contributor Author

All green!

@lucyleeow
Copy link
Contributor

found we're still only requiring 3.7 so I bumped that to 3.8 in a couple of places

Could we add to our release notes that we are dropping support for 3.7?

Copy link
Contributor

@lucyleeow lucyleeow left a comment

Choose a reason for hiding this comment

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

Thank you for fixing the CI failure!

sphinx_gallery/gen_rst.py Outdated Show resolved Hide resolved
sphinx_gallery/gen_rst.py Outdated Show resolved Hide resolved
# memory usage reported ends up being the same.
#
# Modifying an example then adds these two:
# - auto_examples/index
# - auto_examples/plot_numpy_matplotlib
if how == "modify":
n_ch = "[3-7]"
n_ch = "([3-9]|10)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not [3-10] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had that originally! But it's bad regex :) [3-9] is a character range. If you write [3-10] regex complains because it sees this as "the range of characters from 3-1, and also the character zero" -- and the range 3-1 doesn't make sense (it would need to be 1-3 and it's also not what we want). So by doing ([3-9]|10) it will match any single digit between 3 and 9, or the literal string 10, which is what we want.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes of course, thanks!

Co-authored-by: Lucy Liu <jliu176@gmail.com>
@larsoner larsoner enabled auto-merge (squash) September 21, 2023 13:48
@larsoner
Copy link
Contributor Author

I committed your tweaks so I'll mark for merge, thanks for the quick review @lucyleeow

@larsoner larsoner merged commit 528e80d into sphinx-gallery:master Sep 21, 2023
17 checks passed
@larsoner larsoner deleted the write_all branch September 21, 2023 15:24
clrpackages pushed a commit to clearlinux-pkgs/pypi-sphinx_gallery that referenced this pull request Nov 28, 2023
… to version 0.15.0

v0.15.0
-------

Support for Python 3.7 dropped in this release. Requirement is now Python >=3.8.
Pillow added as a dependency.

**Implemented enhancements:**

-  ENH: Improve logging visibility of errors and filenames `#1225 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1225>`__ (`larsoner <https://github.com/larsoner>`__)
-  ENH: Improve API usage graph `#1203 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1203>`__ (`larsoner <https://github.com/larsoner>`__)
-  ENH: Always write sg_execution_times and make DataTable `#1198 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1198>`__ (`larsoner <https://github.com/larsoner>`__)
-  ENH: Write all computation times `#1197 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1197>`__ (`larsoner <https://github.com/larsoner>`__)
-  ENH: Support source files in any language `#1192 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1192>`__ (`speth <https://github.com/speth>`__)
-  FEA Add examples recommender system `#1125 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1125>`__ (`ArturoAmorQ <https://github.com/ArturoAmorQ>`__)

(NEWS truncated at 15 lines)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants