smarie / mkdocs-gallery

Same features as sphinx-gallery (https://sphinx-gallery.github.io/) but on mkdocs (https://www.mkdocs.org/) (no sphinx dependency !).
https://smarie.github.io/mkdocs-gallery
BSD 3-Clause "New" or "Revised" License
38 stars 16 forks source link

Fix attempt: MySubConfig has no '_pre_validate' #59

Closed GenevieveBuckley closed 1 year ago

GenevieveBuckley commented 1 year ago

(maybe) closes https://github.com/smarie/mkdocs-gallery/issues/57

The way MySubConfig is currently implemented is incompatible with mkdocs versions above 1.2.4. I think getting rid of it entirely removes the issue?

It's also not entirely clear to me why the MySubConfig class exists. It looks like there are two reasons:

  1. So you can return an empty dict instead of default values if no info is provided by the user
    • The docstring says "Same as SubConfig except that it will be an empty dict when nothing is provided by user, instead of a dict with all options containing their default values." see this commit (it's a big commit, you have to scroll down to see the relevant part in plugin.py).
    • But I don't know why that's important, or what could go wrong if we take it out. Maybe you can weigh in @smarie?
  2. There is also a fix compensating for a bug in mkdocs see this commit, which you later fixed upstream here. I think that means we don't need that part here now?
smarie commented 1 year ago

Thanks @GenevieveBuckley ! Sorry for the delay with my responses. I triggered the CI. I'll have a look at both your PRs in the upcoming week hopefully

GenevieveBuckley commented 1 year ago

Hmm, test_minimal_conf is failing:

Expand for details: ``` =================================== FAILURES =================================== ______________________________ test_minimal_conf _______________________________ basic_mkdocs_config = {'config_file_path': '/tmp/pytest-of-runner/pytest-0/test_minimal_conf0/mkdocs.yml', 'site_name': 'basic_conf', 'nav':...a': {}, 'plugins': {'search': }, 'hooks': {}, 'watch': []} def test_minimal_conf(basic_mkdocs_config): """Test that minimal config can be loaded without problem""" # Create a mini config mini_config = yaml_load(""" examples_dirs: docs/examples # path to your example scripts gallery_dirs: docs/generated/gallery # where to save generated gallery """) # Load it (this triggers validation and default values according to the plugin config schema) plugin = GalleryPlugin() plugin.load_config(mini_config) # Now mimic the on_config event > result = plugin.on_config(basic_mkdocs_config) /home/runner/work/mkdocs-gallery/mkdocs-gallery/tests/test_config.py:47: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ /home/runner/work/mkdocs-gallery/mkdocs-gallery/.nox/tests-3-9/lib/python3.9/site-packages/mkdocs_gallery/plugin.py:189: in on_config self.config = parse_config(self.config, mkdocs_conf=config) /home/runner/work/mkdocs-gallery/mkdocs-gallery/.nox/tests-3-9/lib/python3.9/site-packages/mkdocs_gallery/gen_gallery.py:134: in parse_config gallery_conf = _complete_gallery_conf(gallery_conf, mkdocs_conf=mkdocs_conf, check_keys=check_keys) /home/runner/work/mkdocs-gallery/mkdocs-gallery/.nox/tests-3-9/lib/python3.9/site-packages/mkdocs_gallery/gen_gallery.py:381: in _complete_gallery_conf gallery_conf['binder'] = check_binder_conf(gallery_conf['binder']) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ binder_conf = {'binderhub_url': 'https://mybinder.org/', 'branch': 'gh-pages', 'dependencies': None, 'filepath_prefix': None, ...} def check_binder_conf(binder_conf): """Check to make sure that the Binder configuration is correct.""" # Grab the configuration and return None if it's not configured binder_conf = {} if binder_conf is None else binder_conf if not isinstance(binder_conf, dict): raise ConfigError('`binder_conf` must be a dictionary or None.') if len(binder_conf) == 0: return binder_conf # Ensure all fields are populated req_values = ['binderhub_url', 'org', 'repo', 'branch', 'dependencies'] optional_values = ['filepath_prefix', 'notebooks_dir', 'use_jupyter_lab'] missing_values = [] for val in req_values: if binder_conf.get(val) is None: missing_values.append(val) if len(missing_values) > 0: > raise ConfigError(f"binder_conf is missing values for: {missing_values}") E mkdocs_gallery.errors.ConfigError: binder_conf is missing values for: ['org', 'repo', 'dependencies']
GenevieveBuckley commented 1 year ago

When I try to run the nox tests, I get an unhashable type: 'list' error. But the same if/then/else check works well for me in an interactive ipython session, and I can't see which object still thinks its a python list. EDIT: nevermind, figured it out!

GenevieveBuckley commented 1 year ago

The nox tests pass for me now, I think this is a decent fix.

smarie commented 1 year ago

I think that I found a solution that is more in the spirit of the new mkdocs configuration mechanism: https://github.com/smarie/mkdocs-gallery/pull/60

Therefore we can probably close this PR now. Thanks a lot @GenevieveBuckley for pointing me in the right direction !

GenevieveBuckley commented 1 year ago

I think that I found a solution that is more in the spirit of the new mkdocs configuration mechanism: #60

Therefore we can probably close this PR now. Thanks a lot @GenevieveBuckley for pointing me in the right direction !

Wonderful! Thank you so much for finding a better fix for this 😄