-
Notifications
You must be signed in to change notification settings - Fork 110
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
NF: register_config() #6601
NF: register_config() #6601
Conversation
+1 on
but in principle this functionality is not needing #6594, right? i.e. extension can only define config variable it uses, thus would need to be imported by then one way (as via |
Feedback: I like it. |
Can be seen in action here: https://github.com/datalad/datalad-next/blob/master/datalad_next/__init__.py (running across platforms) |
#6594 is already needed to give the |
Performance impact when this feature is unused is negligible. Otherwise it is determined by the import time of a particular extensions `__init__.py`. Fixes #datalad#6589
I read the docstring to figure out the proposed mechanism. Looks clear to me. |
This also replaces the previously very flexible data structure for such items with a more rigid set of classes that implement lazy evaluation of defaults (avoiding their possibly expensive computation, even when they are not actually needed). They otherwise implement all elements of the previously used API, such that they work as a drop-in replacement. Only register_config() is intended to be used publicly. Fixed datalad#6314
Code Climate has analyzed commit dcb749e and detected 2 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
I took this out of draft mode. Test coverage is now 100%, and missing functionality for the doc builder was added. I also added |
Restarted CI, since unrelated failures should be gone (except for snapshot build) |
Codecov Report
@@ Coverage Diff @@
## master #6601 +/- ##
==========================================
+ Coverage 91.13% 91.31% +0.17%
==========================================
Files 350 353 +3
Lines 44232 45580 +1348
==========================================
+ Hits 40312 41621 +1309
- Misses 3920 3959 +39
Continue to review full report at Codecov.
|
Everyone in agreement it seems. Merging. |
This PR requires and sits on top of #6594. The first commit is not unique to this PR.
This also replaces the previously very flexible data structure for such items with a more rigid set of classes that implement lazy evaluation of defaults (avoiding their possibly expensive computation, even when they are not actually needed). They otherwise implement all elements of the previously used API, such that they work as a drop-in replacement.
Only register_config() (and its friend
has_config()
is intended to be used publicly. Like so:Changelog
💫 Enhancements and new features
datalad.support.extensions
offers the utility functionsregister_config()
andhas_config()
that allow extension developers to announce additional configuration items to the central configuration management. Fixes formalize extending common_cfg.definitions (and other things) in extensions? #6314