Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom

Fix attempt: MySubConfig has no '_pre_validate' #59

Closed
wants to merge 3 commits into from
Closed

Fix attempt: MySubConfig has no '_pre_validate' #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d109fb7
Fix attempt: MySubConfig has no '_pre_validate'
GenevieveBuckley Mar 30, 2023
dab66f7
Binder check should ignore default-only values
GenevieveBuckley Apr 3, 2023
e1816e2
Fix binder conf values check for configs containing list values
GenevieveBuckley Apr 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/mkdocs_gallery/binder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,19 @@ def _copy_binder_notebooks(gallery_conf, mkdocs_conf):
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):
if not isinstance(binder_conf, (dict, None)):
raise ConfigError('`binder_conf` must be a dictionary or None.')

# Set empty dict if user did not provide any configuration values
if binder_conf is None:
binder_conf = {}
vals = binder_conf.values()
default_values = [None, 'gh-pages', 'https://mybinder.org']
if vals == []:
binder_conf = {}
elif all([val in default_values for val in vals]):
Copy link
Owner

@smarie smarie Apr 11, 2023 •
edited

Choose a reason for hiding this comment

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

This is not very satisfying. What if someone sets 'branch' to 'https://mybinder.org' ? Also it duplicates the information in GalleryPlugin . I would rather find a better way to do this with mkdocs: If you can find your way through mkdocs config class to create a "fixed" MySubConfig it would probably be better.

I'll try to have a look one of these days ; but as you could notice I have a hard time finding a few minutes to code :( sorry @GenevieveBuckley

binder_conf = {}
# Return an empty dict if it's not configured
if len(binder_conf) == 0:
return binder_conf

Expand Down
26 changes: 1 addition & 25 deletions src/mkdocs_gallery/plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,30 +54,6 @@ def run_validation(self, value):
return result


class MySubConfig(co.SubConfig):
"""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."""

def validate(self, value):
if value is None or len(value) == 0:
return None
else:
return super(MySubConfig, self).validate(value)

def run_validation(self, value):
"""Fix SubConfig: errors and warnings were not caught

See https://github.com/mkdocs/mkdocs/pull/2710
"""
failed, self.warnings = Config.validate(self)
if len(failed) > 0:
# get the first failing one
key, err = failed[0]
raise ConfigurationError(f"Sub-option {key!r} configuration error: {err}")

return self


class Dir(co.Dir):
"""mkdocs.config.config_options.Dir replacement: returns a pathlib object instead of a string"""
def run_validation(self, value):
Expand Down Expand Up @@ -122,7 +98,7 @@ class GalleryPlugin(BasePlugin):
('expected_failing_examples', ConfigList(File(exists=True))),
('thumbnail_size', ConfigList(co.Type(int), single_elt_allowed=False)),
('min_reported_time', co.Type(int)),
('binder', MySubConfig(
('binder', co.SubConfig(
# Required keys
('org', co.Type(str, required=True)),
('repo', co.Type(str, required=True)),
Expand Down