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

BF+ENH: search --show-keys -- no incorrect "unhashable", use repr #4354

Merged
merged 7 commits into from Apr 3, 2020

Conversation

yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Mar 27, 2020

+ENH: to be able to limit which keys to report

If you ran smth like datalad search -d /// --show-keys short you could see a long list of keys, where you would see smth like

annex.age
 in  1 datasets
 has 1 unique values: 'unhashable 1688 out of 1690 entries'

which would not only be uninformative but also incorrect. It is 1688 entries which were "hashable" and the rest not. But also you wouldn't see a single value. With first 2 commits (1 commit is the first attempt, superseded by 2nd commit, decided not to squash them) we would use repr for all the values so there is no "unhashable" results reported. With the 3rd commit I added an ENH to be able to limit which keys to report -- a really small/easy addition but quite powerful, so now I can get full result of interest without additional grep or alike:

$> datalad search -d /// --show-keys short '^annex\.age$'
annex.age
 in  1 datasets
 has 1690 unique values: '10'; '10.04'; '10.06'; '10.08'; '10.09'; '10.1'; '10.11'; '10.12'; '10.15'; +1681 values

As you can't see here -- output formatting changed a bit as well -- now uses ';' instead of ','.

NB (edit): I guess it might be more logical to just merge this into master since got a bit too far from a bug fix.

yarikoptic added 5 commits Mar 27, 2020
also changes the format from just adding ... at the end to using ++??? chars++ in
the middle, thus showing the trailer of original repr -- useful if it is a list
or a tuple, so you could see the end of the structure annotation
@codecov
Copy link

codecov bot commented Mar 27, 2020

Codecov Report

Merging #4354 into maint will decrease coverage by 0.01%.
The diff coverage is 92.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #4354      +/-   ##
==========================================
- Coverage   89.67%   89.65%   -0.02%     
==========================================
  Files         275      275              
  Lines       36894    36950      +56     
==========================================
+ Hits        33084    33128      +44     
- Misses       3810     3822      +12     
Impacted Files Coverage Δ
datalad/tests/test_utils.py 96.25% <ø> (ø)
datalad/metadata/search.py 88.47% <90.62%> (+5.10%) ⬆️
datalad/metadata/tests/test_search.py 95.17% <100.00%> (+0.20%) ⬆️
datalad/utils.py 83.97% <100.00%> (-2.67%) ⬇️
datalad/distribution/dataset.py 95.88% <0.00%> (+0.01%) ⬆️
datalad/core/local/create.py 94.77% <0.00%> (+0.03%) ⬆️
datalad/cmd.py 89.68% <0.00%> (+0.05%) ⬆️

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 cc7e627...df5dc90. Read the comment docs.

@yarikoptic yarikoptic added the merge-if-ok label Mar 29, 2020
@yarikoptic
Copy link
Member Author

yarikoptic commented Apr 1, 2020

since it is something I would like to use (and in general - use this functionality), and since nobody else objects and/or cares, I will merge it tomorrow.

Copy link
Collaborator

@kyleam kyleam left a comment

I'm not very familiar with this code, but the changes in addition to the bug fixes look like clear improvements and no obvious issues popped out at me on a read-through of the series.

re: maint vs master: I'd lean towards master with this, but either way is okay by me.

@yarikoptic
Copy link
Member Author

yarikoptic commented Apr 3, 2020

thank you @kyleam ! I am split between maint-vs-master, since ideally I should have just made a BF for maint, but since it is a minor enhancement to rarely used user-interfaced (not machine readable) information, I will just proceed with maint.

@yarikoptic yarikoptic merged commit 86487e1 into datalad:maint Apr 3, 2020
10 of 11 checks passed
@yarikoptic yarikoptic added this to the 0.12.6 milestone Apr 3, 2020
@yarikoptic yarikoptic deleted the enh-list-keys branch May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-if-ok
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants