Skip to content

Conversation

@thomasjpfan
Copy link
Member

This PR uses sphinx-gallery's reset_modules to reset the global config. This way we do not need to manually reset the global config at the end of every example.

CC @lesteve

Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

Nice feature. LGTM.

"inspect_global_variables": False,
"remove_config_comments": True,
"plot_gallery": "True",
"reset_modules": ("matplotlib", "seaborn", reset_sklearn_config),
Copy link
Member

Choose a reason for hiding this comment

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

question: why can't we simply say sklearn here? Also, shouldn't numpy (and other deps maybe) be here too? One could imagine an example where numpy's print options are modified for example.

Copy link
Member

Choose a reason for hiding this comment

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

The "matplotlib" and "seaborn" are sphinx-gallery built-in reset functions. They are not implemented for every module.

@lesteve
Copy link
Member

lesteve commented May 9, 2023

This seems indeed a lot better to do it this way. Let's merge this one, thanks!

@lesteve lesteve merged commit fe52f14 into scikit-learn:main May 9, 2023
@lesteve
Copy link
Member

lesteve commented May 9, 2023

I opened #26350 to tweak a comment in another example that was misleading after this PR was merged, since it was mentioning side-effects on other examples.

REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
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.

5 participants