RF: Simplify EnsureDataset and add optional ID check #279
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously this method only got a `Dataset`-type argument. However, for path resolution (for example), this information is insufficient, because DataLad needs to know whether the type of the original argument. For this reason `DatasetParameter` already existed, and is now also used for this purpose. This also simplified the type-annotation -- no need for custom typevars. It also made sense to move the `DatasetParameter` class to `base.py`, because every single constraint implementation has to support it (or use the default implementation from `base.py`.
This implements DataLad's standard path resolution approach (relative
is relative to CWD, unless a `Dataset` instance is given).
This now makes it possible to perform this standard resolution pattern
during command parameter validation, like so:
```py
\# example validator for a command with two args
EnsureCommandParameterization(
param_constraints=dict(
dataset=EnsureDataset(),
path=EnsurePath(),
),
tailor_for_dataset=dict(path='dataset'),
)
```
This will:
- cause the 'dataset' validator to run first
- auto-generate an `EnsurePath` variant that resolves against that
dataset
- ensures that the command always receives absolute paths, resolved
against the dataset whenever the rules require it.
Closes datalad#270
mih
requested changes
Mar 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This sits on top of #271 to prevent a merge conflict and make use of the changes required for type checking already implemented in it.
This PR adds two changes in individual commits:
The first commit refactors
EnsureDataset- there was some duplication that I thought could be reduced by factoring out a_require_dataset()helper.The second commit adds an optional
idchecktoEnsureDatasetto fix #272