search

datalad / datalad-ukbiobank

Resources for working with UKBiobank as a DataLad dataset
MIT License
6 stars 12 forks source link

Fix two inconveniences re dataset maintenance #56

Closed mih closed 3 years ago

mih commented 4 years ago

See commit messages for details.

Fixes #54 Fixes #55 Fixes #68

codecov[bot] commented 3 years ago

Codecov Report

Merging #56 (a2e3c65) into master (a9070dd) will decrease coverage by 0.21%. The diff coverage is 95.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
- Coverage   97.26%   97.05%   -0.22%     
==========================================
  Files           9        9              
  Lines         329      339      +10     
==========================================
+ Hits          320      329       +9     
- Misses          9       10       +1
Impacted Files Coverage Δ
datalad_ukbiobank/update.py 94.73% <80.00%> (-0.97%) :arrow_down:
datalad_ukbiobank/init.py 100.00% <100.00%> (ø)
datalad_ukbiobank/tests/test_init.py 100.00% <100.00%> (ø)
datalad_ukbiobank/tests/test_update.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a9070dd...a2e3c65. Read the comment docs.

mih commented 3 years ago

This is good now.

loj commented 3 years ago

I tested this out a bit, and a situation that still creates a problem for me is running ukb-update in a fresh clone. It will fail with 

[ERROR  ] Unknown commit identifier: incoming [gitrepo.py:get_hexsha:1847] (ValueError)

Specifically it fails at line 151 (initial_incoming = repo.get_hexsha('incoming'))

The situation I see this coming up in is if you don't need to re-initialize the cloned dataset to add datatypes, and just want to run ukb-update to check for changes to the existing data.

To reproduce:

datalad create sub-1002532
cd sub-1002532
datalad ukb-init --bids 1002532 20227_2_0
datalad ukb-update --keyfile <path_to_key> --merge

cd ../
datalad clone sub-1002532 testclone
cd testclone
datalad ukb-update --keyfile <path_to_key> --merge

Otherwise, the changes to address #54 and #68 look good. :-)

mih commented 3 years ago

Thanks! I have pinpointed the failure with a unittest. Will look for a fix and update this PR.

mih commented 3 years ago

Fix pushed, passes for me locally.

loj commented 3 years ago

Cool! It works for me as well. :-)

mih commented 3 years ago

Alrighty, so let's be done with this! Thx!