ENH Clean-up code by early initialization of sphinx_gallery_conf #1120
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lery into early-config-validation
|
Agreed this should fix the show-api-usage bug, and agree this is much cleaner. I looked at the code and don't see any issues so in it goes, thanks @lesteve ! |
clrpackages
referenced
this pull request
in clearlinux-pkgs/pypi-sphinx_gallery
Apr 18, 2023
… to version 0.13.0 v0.13.0 ------- **Implemented enhancements:** - ENH: Create backreferences for default roles `#1122 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1122>`__ (`StefRe <https://github.com/StefRe>`__) - ENH raise error in check_jupyterlite_conf with unknown key `#1119 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1119>`__ (`lesteve <https://github.com/lesteve>`__) - ENH Add functionality to modify Jupyterlite notebooks based on their content `#1113 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1113>`__ (`lesteve <https://github.com/lesteve>`__) - ENH: Add support for WebP `#1111 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1111>`__ (`StefRe <https://github.com/StefRe>`__) **Fixed bugs:** - ENH Clean-up code by early initialization of sphinx_gallery_conf `#1120 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1120>`__ (`lesteve <https://github.com/lesteve>`__) - FIX JupyterLite button links `#1115 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1115>`__ (`lesteve <https://github.com/lesteve>`__) - Fix thumbnail text formatting `#1108 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1108>`__ (`StefRe <https://github.com/StefRe>`__) (NEWS truncated at 15 lines)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
While working on #977, I noticed that the config initialization happened at
builder-initedtime ingen_gallery.parse_config, but it would have been more convenient for me that it happened atconfig-initedtime because some of the JupyterLite config is used atconfig-inited. I needed to duplicate some logic like default values in codesphinx_gallery_conf.get('jupyterlite', {})and ingen_gallery.DEFAULT_GALLERY_CONF, callcheck_jupyterlite_configin multiple places, etc ...I thought it would be a lot better if the
configcould be "completed" (as we say in the code in some places, I used normalized in other places, but I am open to better naming suggestions ...) once early and we would be able to use it without always wondering whether an optional key exists or not. It would also to get rid of repeated.get(..., default_value)in multiple places.I also believe that it would also potentially avoid the show_api_usage issue seen in #1093 although I am not 100% sure I have to say. It does feel like the
config-initedwill happen before theautodoc-process-docstringevent. I have removed the code from #1095 for now but I can put it back if deemedsafer.
Edit: I tried on my branch with the instructions from #1095 (comment) and the issue doesn't happen on my branch it seems.
Of course things were not quite as simple as I initially hoped, since there is also some configuration update that needs to be done at
builder-initedOverview of the changes
gen_gallery._complete_gallery_confin two function one for config-inited one for builder-initedgen_gallery.normalize_gallery_conf_config_initedandgen_gallery.normalize_gallery_conf_builder_initedthat we connect to theconfig-initedandbuilder-initedeventgen_gallery.parse_configthat we don't need anymoresphinx_gallery_confin a good enough state to be able to test the behavioursphinx_gallery_conf.get(key, default_value)in favour ofsphinx_gallery_conf['key']