Skip to content

MNT Add test for _bool_eval and add configs to _bool_eval check#1442

Merged
larsoner merged 4 commits intosphinx-gallery:masterfrom
lucyleeow:test_bool_eval
Feb 28, 2025
Merged

MNT Add test for _bool_eval and add configs to _bool_eval check#1442
larsoner merged 4 commits intosphinx-gallery:masterfrom
lucyleeow:test_bool_eval

Conversation

@lucyleeow
Copy link
Copy Markdown
Contributor

closes #1411

  • Add test for _bool_eval
  • Adds bool only configurations to _bool_eval check
  • Improves build options section, as I think any configuration can be altered via -D build option, as long as it is one of the accepted types by -D. As we now accept callables as string path to object, I think these can now also be set via -D.

@lucyleeow lucyleeow changed the title MNT Add test for _bool_eval MNT Add test for _bool_eval and add configs to _bool_eval check Feb 27, 2025
your ``Makefile`` with:
website to display, or any use you can imagine for it.

This can be done by setting the ``plot_gallery`` configuration in the
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was a more opinionated change, to talk about the makefile second, but I thought it was more consistent with the rest of the docs which generally talk about setting the conf in the conf.py file first

Copy link
Copy Markdown
Contributor

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Just one idea / question about a comment update

# XXX anything that can only be a bool (rather than str) should probably be
# evaluated this way as it allows setting via -D on the command line
for key in (
"download_all_examples",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we get rid of the XXX comment above now that you've made these changes?

Copy link
Copy Markdown
Contributor Author

@lucyleeow lucyleeow Feb 27, 2025

Choose a reason for hiding this comment

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

Good idea, I've amended the comment just to remind us why we are doing the _bool_eval

@larsoner larsoner merged commit 14597d7 into sphinx-gallery:master Feb 28, 2025
20 checks passed
@larsoner
Copy link
Copy Markdown
Contributor

Thanks @lucyleeow !

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.

Add test for _bool_eval

2 participants