ENH: subdatasets(contains=...) will report matching paths in result #3743
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
Sorting paths (for locally present content, and unavailable stuff) into
containing (present) datasets is a key task for helper code such as
annotate_paths(). This change equips `subdatasets()` with the ability
to make similar reports by including a 'contains' property in its
results whenever a respective set of paths has been provided.
Such a result looks like this:
```
{
"action": "subdataset",
"contains": [
"/tmp/datalad_temp_test_get_in_unavailable_subdatasetdzceqobr/sub1/sub2"
],
"gitmodule_datalad-id": "1b1e4b70-e472-11e9-9214-f0d5bf7b5561",
"gitmodule_name": "sub1",
"gitmodule_url": "./sub1",
"gitshasum": "15428154a3573488503151ddc42051281221e8f8",
"parentds": "/tmp/datalad_temp_test_get_in_unavailable_subdatasetdzceqobr",
"path": "/tmp/datalad_temp_test_get_in_unavailable_subdatasetdzceqobr/sub1",
"refds": "/tmp/datalad_temp_test_get_in_unavailable_subdatasetdzceqobr",
"revision": "15428154a3573488503151ddc42051281221e8f8",
"state": "absent",
"status": "ok",
"type": "dataset"
}
```
where paths are reported as a 'contains' list with fully resolved items.
This is improves its utility for sorting parts into datasets further. Anything that doesn't match a subdataset must either be part of the existing base dataset, or does not exist at all.
Codecov Report
@@ Coverage Diff @@
## master #3743 +/- ##
===========================================
- Coverage 82.98% 60.83% -22.15%
===========================================
Files 273 273
Lines 35918 35933 +15
===========================================
- Hits 29807 21861 -7946
- Misses 6111 14072 +7961
Continue to review full report at Codecov.
|
kyleam
approved these changes
Oct 1, 2019
| if not contains_hits: | ||
| # we are not looking for this subds, because it doesn't | ||
| # match the target path | ||
| continue |
There was a problem hiding this comment.
So the main price we pay for this added information is not aborting the for c in contains iteration early if one of the elements is a hit. OK.
If an argument given to `contains` doesn't have a subdataset match, subdatasets() now yields a record with status="impossible" rather than nothing.
kyleam
force-pushed
the
enh-subdatasets
branch
from
7f1eed9
to
f634bea
Compare
|
appveyor failure from the known stalling of |
|
Thx @kyleam for the review and this fix! I will keep this open for now, until I know it is suitable for use in |
|
Ok, it can go in now. |
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.
Sorting paths (for locally present content, and unavailable stuff) into
containing (present) datasets is a key task for helper code such as
annotate_paths(). This change equips
subdatasets()with the abilityto make similar reports by including a 'contains' property in its
results whenever a respective set of paths has been provided.
Such a result looks like this:
where paths are reported as a 'contains' list with fully resolved items.
This seems like a useful feature on its own, but is part of a move towards fixing #3368 and #3469