-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
More gracefully override JPS ExtensionApp.config_file_paths
#612
Conversation
ExtensionApp.config_file_paths
ExtensionApp.config_file_paths
ExtensionApp.config_file_paths
@property | ||
def config_file_paths(self) -> List[str]: | ||
if self._config_file_paths is None: | ||
ret = get_conf_dir_hierarchy( | ||
[ | ||
SITE_CONF_ROOT, # site configuration | ||
USER_CONF_ROOT, # user configuration | ||
], filename=False | ||
) | ||
# Next include currently needed for directory making | ||
ret.insert(0, str(Path(uis_pkg).parent)) # packaged config | ||
ret.reverse() | ||
self._config_file_paths = ret | ||
return self._config_file_paths |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change does not appear to be necessary, the only bit that's needed is the autouse
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't get the monkeypatching to work without this change. Not sure why, but pyright is not happy with overriding a property with a class attribute anyway (see microsoft/pyright#3646 (comment))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Monkeypatching seems to work fine for me (with your config)?
I found the config_file_paths
definition in Jupyter_Core and it is indeed a property so this makes sense 👍.
This allows patching in the tests so the tests don't pick up our user/site configs. Currently the tests fail locally for me because they pick up my config.