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.

Sign up for GitHub

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

Jump to bottom

[AL-1698] Sample info #1550

Merged
merged 87 commits into from
Mar 30, 2022
Merged

[AL-1698] Sample info #1550

merged 87 commits into from
Mar 30, 2022

Conversation

farizrahman4u
Copy link
Contributor

🚀 🚀 Pull Request

Checklist:

  • My code follows the style guidelines of this project and the Contributing document
  • I have commented my code, particularly in hard-to-understand areas
  • I have kept the coverage-rate up
  • I have performed a self-review of my own code and resolved any problems
  • I have checked to ensure there aren't any other open Pull Requests for the same change
  • I have described and made corresponding changes to the relevant documentation
  • New and existing unit tests pass locally with my changes

Changes

@farizrahman4u
init
9367a1c
@farizrahman4u
Merge branch 'main' of https://www.github.com/activeloopai/hub into f…
6870c6e 
…r_partial_upload
@farizrahman4u
partial upload
7ebb58b
@farizrahman4u
test fixes
e52d415
@farizrahman4u
mypy
0452a21
@farizrahman4u
merge main
b382e64
@farizrahman4u
merge main
54ebb01
@farizrahman4u
more test
fe0f102
@farizrahman4u
Update __init__.py
5a21814
@farizrahman4u
refac + tests
3846862
@farizrahman4u
Merge branch 'fr_partial_upload' of https://www.github.com/activeloop…
c41a99e 
…ai/hub into fr_partial_upload
@farizrahman4u
rem debg line
ee21646
@farizrahman4u
Merge branch 'main' into fr_partial_upload
27a314c
@farizrahman4u
sequence htype
d8eec0f
@farizrahman4u
Merge branch 'fr_partial_upload' into fr_sequence_htype
0384067
@farizrahman4u
fix
1f5d7f3
@farizrahman4u
Merge branch 'fr_sequence_htype' of https://www.github.com/activeloop…
b0a5c01 
…ai/hub into fr_sequence_htype
@farizrahman4u
test
e40ca57
@farizrahman4u
update test
846838e
@farizrahman4u
fix updates
5ef3f8c
@farizrahman4u
typo
d280215
@farizrahman4u
merge main
ffe8059
@farizrahman4u
fix double indexing
8e96cff
@farizrahman4u
format
2db488a
@farizrahman4u
test shape bug
29180b5
@farizrahman4u
format
95f2ae9
@farizrahman4u
compression tests
fbe28ae
@farizrahman4u
fix test_like
e4d6c45
@farizrahman4u
format
5ccec73
@farizrahman4u
hub.read + sequence tests
8ef00da
@farizrahman4u farizrahman4u changed the base branch from fr_linked_tensors to main March 24, 2022 12:43
@farizrahman4u farizrahman4u marked this pull request as ready for review March 24, 2022 12:43
AbhinavTuli
AbhinavTuli reviewed Mar 24, 2022
View reviewed changes
hub/core/sample.py
@@ -354,5 +380,31 @@ def _read_from_gcs(self) -> bytes:
def _read_from_http(self) -> bytes:
return urlopen(self.path).read() # type: ignore

def _getexif(self) -> dict:
Copy link
Contributor

Choose a reason for hiding this comment

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

docstring

AbhinavTuli
AbhinavTuli reviewed Mar 24, 2022
View reviewed changes
hub/core/tensor.py
if sample_info_tensor is None:
return None
if index.subscriptable_at(0):
return list(
Copy link
Contributor

Choose a reason for hiding this comment

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

kinda hard to parse

AbhinavTuli
AbhinavTuli approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@AbhinavTuli AbhinavTuli left a comment

Choose a reason for hiding this comment

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

Some small comments, rest looks good to merge.

@codecov
Copy link

codecov bot commented Mar 24, 2022 •
edited
Loading

Codecov Report

Merging #1550 (5bf0510) into main (163483c) will decrease coverage by 0.03%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##             main    #1550      +/-   ##
==========================================
- Coverage   92.36%   92.33%   -0.04%     
==========================================
  Files         199      200       +1     
  Lines       17950    18167     +217     
==========================================
+ Hits        16580    16774     +194     
- Misses       1370     1393      +23
Flag Coverage Δ
unittests 92.33% <90.90%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
hub/api/tests/test_sample_info.py 85.54% <85.54%> (ø)
hub/core/tensor.py 84.89% <86.66%> (-0.04%) ⬇️
hub/core/chunk_engine.py 94.61% <90.00%> (-0.42%) ⬇️
hub/core/tensor_link.py 93.44% <92.30%> (-1.01%) ⬇️
hub/core/compression.py 87.90% <94.73%> (+0.15%) ⬆️
hub/core/sample.py 89.91% <96.87%> (+1.03%) ⬆️
hub/api/tests/test_api.py 100.00% <100.00%> (ø)
hub/constants.py 100.00% <100.00%> (ø)
hub/core/dataset/dataset.py 92.86% <100.00%> (+0.13%) ⬆️
hub/core/index/index.py 84.77% <100.00%> (ø)
... and 3 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 163483c...5bf0510. Read the comment docs.

@farizrahman4u farizrahman4u merged commit 69c85f1 into main Mar 30, 2022
@farizrahman4u farizrahman4u deleted the fr_sample_meta2 branch March 30, 2022 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FayazRahman FayazRahman FayazRahman approved these changes

@AbhinavTuli AbhinavTuli AbhinavTuli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@farizrahman4u @FayazRahman @AbhinavTuli