Skip to content
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: Add alias argument to create_sibling_ria #5592

Merged
merged 6 commits into from
Apr 28, 2021

Conversation

mikapfl
Copy link
Contributor

@mikapfl mikapfl commented Apr 21, 2021

Overview

Add an argument to create_sibling_ria to add an alias symlink to the RIA store to refer to this dataset.

TODO

  • Integration test using create_sibling_ria directly
  • add to .zenodo.json

Comment

Would be great to know if the way I'm adding this functionality looks sane, it is the first time I touch the datalad code.

@codecov
Copy link

codecov bot commented Apr 21, 2021

Codecov Report

Merging #5592 (3af15e4) into master (b29c58a) will decrease coverage by 1.80%.
The diff coverage is 98.55%.

❗ Current head 3af15e4 differs from pull request most recent head 64fbe66. Consider uploading reports for the commit 64fbe66 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5592      +/-   ##
==========================================
- Coverage   90.29%   88.48%   -1.81%     
==========================================
  Files         304      301       -3     
  Lines       41452    41502      +50     
==========================================
- Hits        37429    36723     -706     
- Misses       4023     4779     +756     
Impacted Files Coverage Δ
datalad/distributed/create_sibling_ria.py 82.65% <ø> (ø)
datalad/distributed/ora_remote.py 31.81% <85.71%> (+0.64%) ⬆️
datalad/customremotes/ria_utils.py 93.65% <100.00%> (+1.05%) ⬆️
...talad/distributed/tests/test_create_sibling_ria.py 100.00% <100.00%> (ø)
datalad/distributed/tests/test_ria_basics.py 97.67% <100.00%> (+0.20%) ⬆️
datalad/plugin/wtf.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/addurls.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/no_annex.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/add_readme.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/check_dates.py 0.00% <0.00%> (-100.00%) ⬇️
... and 61 more

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 b29c58a...64fbe66. Read the comment docs.

Copy link
Member

@bpoldrack bpoldrack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for approaching this, @mikapfl!

Would be great to know if the way I'm adding this functionality looks sane, it is the first time I touch the datalad code.

Yes, this looks perfectly sane. :)

datalad/customremotes/ria_utils.py Outdated Show resolved Hide resolved
@mih
Copy link
Member

mih commented Apr 22, 2021

This is very useful! Thx a lot!

@mih mih changed the title NF: Add alis argument to create_sibling_ria NF: Add alias argument to create_sibling_ria Apr 22, 2021
@mikapfl
Copy link
Contributor Author

mikapfl commented Apr 22, 2021

I've added a warning if the alias is already taken and tests for that. I'm not quite sure what's up with the benchmarks run or why codecov complains, I think these are unrelated to my changes.

@mikapfl mikapfl marked this pull request as ready for review April 22, 2021 13:59
@yarikoptic
Copy link
Member

unrelated by promoted by this PR, @mikapfl please react to datalad/datalad-paper-joss#93

@yarikoptic
Copy link
Member

@bpoldrack @mih, please bless this PR -- I am not familiar with ria code to make a judgement call on either anything else is needed to get this merged

Copy link
Member

@mih mih left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx again, LGTM! I will merge this now. Over the next two days, I will give it a thorough real-world test ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants