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

OPT+RF: create-sibling --since=^ -- diff to HEAD, use "^" instead of "" #6436

Merged
merged 2 commits into from
Feb 10, 2022

Conversation

yarikoptic
Copy link
Member

Two commits:

OPT: create_sibling - diff to HEAD not None (tree)

Primary motivation is very slow --since= mode of operation of create-sibling
on a heavy superdataset (see #6399).  Although it could be argued that there might be
a benefit from create-sibling to be able to create siblings for not yet committed
submodules, I think such use cases are sparse if any and thus diff"ing to the committed
state (HEAD) is actually the desired effect.

Fixes #6399 (although I did not check explicitly)

RF: create-sibling --since to take "^" instead of ""

to be consistent with "push" and thus reduce cognitive load.  Old value of an
empty string will remain supported.  While at it, added a tiny bit of testing
to ensure that --since actually works out the way we expect and only deals with
that subdataset (or notneeded)

Moreover, apparently --since= was not even documented before, this fixes it.

First commit I would not even mind to destine against maint but decided not to bother since 0.16.0 is imminent, right?

Changelog

💫 Enhancements and new features

  • create-sibling --since=^ mode will now be as fast as push --since=^ to figure out for which subdatasets to create siblings.

🪓 Deprecations and removals

  • create-sibling will require now "^" instead of an empty string for since option.

📝 Documentation

  • --since=^ mode of operation of create-sibling is documented now

Primary motivation is very slow --since= mode of operation of create-sibling
on a heavy superdataset (see datalad#6399).  Although it could be argued that there might be
a benefit from create-sibling to be able to create siblings for not yet committed
submodules, I think such use cases are sparse if any and thus diff"ing to the committed
state (HEAD) is actually the desired effect.

Fixes datalad#6399 (although I did not check explicitly)
to be consistent with "push" and thus reduce cognitive load.  Old value of an
empty string will remain supported.  While at it, added a tiny bit of testing
to ensure that --since actually works out the way we expect and only deals with
that subdataset (or notneeded)
@yarikoptic yarikoptic added performance Improve performance of an existing feature semver-minor Increment the minor version when merged labels Feb 9, 2022
@codeclimate
Copy link

codeclimate bot commented Feb 9, 2022

Code Climate has analyzed commit fe60cd4 and detected 0 issues on this pull request.

View more on Code Climate.

@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #6436 (fe60cd4) into master (b868dbc) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6436      +/-   ##
==========================================
+ Coverage   89.87%   89.95%   +0.08%     
==========================================
  Files         348      348              
  Lines       43764    43772       +8     
==========================================
+ Hits        39331    39377      +46     
+ Misses       4433     4395      -38     
Impacted Files Coverage Δ
datalad/distribution/create_sibling.py 65.14% <100.00%> (+0.40%) ⬆️
datalad/distribution/tests/test_create_sibling.py 77.57% <100.00%> (+0.15%) ⬆️
datalad/customremotes/tests/test_archives.py 89.28% <0.00%> (-0.65%) ⬇️
datalad/support/tests/test_annexrepo.py 97.97% <0.00%> (+0.13%) ⬆️
datalad/distributed/tests/test_ria_basics.py 97.91% <0.00%> (+0.29%) ⬆️
datalad/distributed/ora_remote.py 31.57% <0.00%> (+0.32%) ⬆️
datalad/local/add_archive_content.py 88.13% <0.00%> (+0.42%) ⬆️
datalad/local/rerun.py 96.41% <0.00%> (+0.65%) ⬆️
datalad/runner/tests/test_witless_runner.py 95.39% <0.00%> (+0.65%) ⬆️
datalad/downloaders/base.py 82.47% <0.00%> (+0.68%) ⬆️
... and 6 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 b868dbc...fe60cd4. Read the comment docs.

@yarikoptic
Copy link
Member Author

It is magically green! Even nearly all benchmarks under 1 - can't be bad ;-)

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.

Let's not let this unicorn event slip!

@yarikoptic
Copy link
Member Author

note FTR: damn, burnt myself -- thought that it was against maint, spent time tracking down in "maint". uff.

@yarikoptic yarikoptic deleted the enh-create-sibling-since branch April 5, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Improve performance of an existing feature semver-minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants