-
Notifications
You must be signed in to change notification settings - Fork 10
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
Patch core's configuration
to allow get
from global scope
#86
Conversation
...without requiring a dataset to be present. This patch replaces both frontend functions for `configuration`, because the input validation checks are not sufficiently modularized for a more surgical patch. Two key changes are made: 1) The unnecessary prevention of a `get` without a dataset around is removed, and the documentation is adjusted. 2) The `configuration()` function now selects the appropriate config manager based on the `ds` argument value alone, rather than implementing additional cross-comparison with the `scope` argument. Such argument validation is not done in `Configuration.__call__()` alone. Closes datalad/datalad#6854
Codecov Report
@@ Coverage Diff @@
## main #86 +/- ##
==========================================
- Coverage 93.24% 87.95% -5.30%
==========================================
Files 28 30 +2
Lines 2103 2474 +371
==========================================
+ Hits 1961 2176 +215
- Misses 142 298 +156
Continue to review full report at Codecov.
|
cool, this works for me 👍 |
cfg.reload(force=True) | ||
|
||
|
||
conf_mod.Configuration.__call__ = Configuration.__call__ |
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.
If you derive from from conf_mod.Configuration
, wouldn't it make sense to replace the class rather than just __call__
? Or the other way around: derive from Interface
as usual?
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.
I thought so too, but just replacing the class is not enough. You would still need to replace the references to its __call__
. When I did this, I ended up in a situation where replacing the class itself no longer has any impact.
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.
I seem to remember I got that way of patching to work at some point. Need to dig. But okay - don't let it be an obstacle for merge. We can "fix" that later.
get
from global scopeconfiguration
to allow get
from global scope
...without requiring a dataset to be present.
This patch replaces both frontend functions for
configuration
, becausethe input validation checks are not sufficiently modularized for a
more surgical patch. Two key changes are made:
get
without a dataset around isremoved, and the documentation is adjusted.
configuration()
function now selects the appropriate configmanager based on the
ds
argument value alone, rather thanimplementing additional cross-comparison with the
scope
argument.Such argument validation is not done in
Configuration.__call__()
alone.
Closes datalad/datalad#6854