Skip to content

ENH: Write all computation times#1197

Merged
larsoner merged 8 commits intosphinx-gallery:masterfrom
larsoner:write_all
Sep 21, 2023
Merged

ENH: Write all computation times#1197
larsoner merged 8 commits intosphinx-gallery:masterfrom
larsoner:write_all

Conversation

@larsoner
Copy link
Copy Markdown
Contributor

@larsoner larsoner commented Sep 19, 2023

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.

lucyleeow
lucyleeow approved these changes Sep 20, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@lucyleeow lucyleeow left a comment •
edited
Loading

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:

sphinx-gallery/doc/getting_started.rst

Line 150 in 11812e6

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

about the root sg_execution_times.rst file?

sphinx_gallery/tests/test_full.py Outdated
# - auto_examples/plot_numpy_matplotlib
if how == "modify":
n_ch = "[3-7]"
n_ch = "[3-9]"
Copy link
Copy Markdown
Contributor

@lucyleeow lucyleeow Sep 20, 2023

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
Copy Markdown
Contributor Author

@larsoner larsoner Sep 20, 2023

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
Copy Markdown
Contributor Author

@larsoner larsoner Sep 20, 2023

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
Copy Markdown
Contributor Author

larsoner commented Sep 20, 2023

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
Copy Markdown
Contributor Author

larsoner commented Sep 20, 2023

All green!

@lucyleeow
Copy link
Copy Markdown
Contributor

lucyleeow commented Sep 20, 2023

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?

lucyleeow
lucyleeow approved these changes Sep 20, 2023
View reviewed changes
Copy link
Copy Markdown
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/tests/test_full.py
# - auto_examples/plot_numpy_matplotlib
if how == "modify":
n_ch = "[3-7]"
n_ch = "([3-9]|10)"
Copy link
Copy Markdown
Contributor

@lucyleeow lucyleeow Sep 20, 2023

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
Copy Markdown
Contributor Author

@larsoner larsoner Sep 21, 2023

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
Copy Markdown
Contributor

@lucyleeow lucyleeow Sep 21, 2023

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!

@larsoner larsoner enabled auto-merge (squash) September 21, 2023 13:48
@larsoner
Copy link
Copy Markdown
Contributor Author

larsoner commented Sep 21, 2023

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

@larsoner larsoner disabled auto-merge September 21, 2023 13:52
@larsoner larsoner merged commit 528e80d into sphinx-gallery:master Sep 21, 2023
@larsoner larsoner deleted the write_all branch September 21, 2023 15:24
@lucyleeow lucyleeow mentioned this pull request Sep 22, 2023
Merged
clrpackages referenced this pull request in clearlinux-pkgs/pypi-sphinx_gallery Nov 28, 2023
pypi-sphinx_gallery: Autospec creation for update from version 0.14.0…
7799fb9 
… 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

Reviewers

@lucyleeow lucyleeow lucyleeow approved these changes

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@larsoner @lucyleeow